[lkml]   [2010]   [May]   [23]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
    SubjectRe: [PATCH 14/27] drivers/scsi: Use memdup_user
    On Sun, 2010-05-23 at 18:36 +0300, Boaz Harrosh wrote:
    > > This type of transformation has really no value at all. The code you're
    > > proposing to replace is already correct. I'm fairly ambivalent on
    > > patterned APIs anyway but I accept they're useful way to prevent new
    > > code getting it wrong. However, it's completely bogus to force
    > > replacement of correctly functioning code throughout the kernel (unless
    > > you're going to argue that everyone who tries to copy from user into a
    > > kmalloc space does a cut and paste from sg?)
    > >
    > > Of infinitely greater service would be finding any places where the
    > > pattern is being incorrectly used.
    > >
    > It looks like it is not done 100% kosher and calling memdup_user should
    > be better.
    > - For 1 memdup_user does a GFP_KERNEL with a comment on how copy_from_user
    > would eventually sleep, so what's the point of GFP_ATOMIC?

    Well, since you've written a storage driver, I really hope that question
    is rhetorical ...

    The reason for using GFP_ATOMIC from user context in storage drivers is
    to avoid writeout deadlock: you don't want to trigger a swap write
    while you potentially occupy the writeout path. In all older drivers
    this had to be GFP_ATOMIC because GFP_NOIO wasn't around.

    This also illustrates the problem with design patterns: The idea that
    if you have user context, you must be able to kmalloc GFP_KERNEL seems
    logical to the people who wrote the pattern, but actually it's
    potentially incorrect for storage.

    Now in the particular case of sg, I don't believe we'll ever want to
    swap over sg (famous last words, of course), so in this instance we
    probably could get away with using GFP_KERNEL ... but since it's
    following the storage pattern, GFP_ATOMIC (or GFP_NOIO) is correct.

    Does osd need auditing for this problem (or would no-one ever do swap
    over osd)?

    > If this is by design then it surly calls for a comment that explains.
    > (I would like to know)

    This pattern occurs many times in storage ... documenting it at every
    callsite would be a huge waste.


     \ /
      Last update: 2010-05-23 18:25    [W:0.023 / U:33.720 seconds]
    ©2003-2017 Jasper Spaans. hosted at Digital OceanAdvertise on this site