lkml.org 
[lkml]   [2023]   [Apr]   [25]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    SubjectRe: [PATCH v3 41/55] iscsi: Assume "sendpage" is okay in iscsi_tcp_segment_map()
    Date
    Fabio M. De Francesco <fmdefrancesco@gmail.com> wrote:

    > > - if (recv) {
    > > - segment->atomic_mapped = true;
    > > - segment->sg_mapped = kmap_atomic(sg_page(sg));
    > > - } else {
    > > - segment->atomic_mapped = false;
    > > - /* the xmit path can sleep with the page mapped so use
    > kmap */
    > > - segment->sg_mapped = kmap(sg_page(sg));
    > > - }
    > > -
    > > + segment->atomic_mapped = true;
    > > + segment->sg_mapped = kmap_atomic(sg_page(sg));
    >
    > As you probably know, kmap_atomic() is deprecated.
    >
    > I must admit that I'm not an expert of this code, however, it looks like the
    > mapping has no need to rely on the side effects of kmap_atomic() (i.e.,
    > pagefault_disable() and preempt_disable() - but I'm not entirely sure about
    > the possibility that preemption should be explicitly disabled along with the
    > replacement with kmap_local_page()).
    >
    > Last year I've been working on several conversions from kmap{,_atomic}() to
    > kmap_local_page(), however I'm still not sure to understand what's happening
    > here...
    >
    > Am I missing any important details? Can you please explain why we still need
    > that kmap_atomic() instead of kmap_local_page()?

    Actually, it might be worth dropping segment->sg_mapped and segment->data and
    only doing the kmap_local when necessary.

    And this:

    struct msghdr msg = { .msg_flags = flags };
    struct kvec iov = {
    .iov_base = segment->data + offset,
    .iov_len = copy
    };

    r = kernel_sendmsg(sk, &msg, &iov, 1, copy);

    should really be using struct bvec, not struct kvec - then the mapping isn't
    necessary. It looks like this might be the only place the mapping is used,
    but I'm not 100% certain.

    David

    \
     
     \ /
      Last update: 2023-04-25 10:42    [W:6.474 / U:0.044 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site