Messages in this thread | | | Date | Fri, 22 Sep 2006 10:36:40 +0200 | From | Jiri Slaby <> | Subject | Re: [KJ] kmalloc to kzalloc patches for drivers/block [sane version] |
| |
Om Narasimhan wrote: > Thanks for the comments. >> > >> > Signed off by Om Narasimhan <om.turyx@gmail.com> >> >> This is not the canonical format, per SubmittingPatches. It should be: >> >> Signed-off-by: Random J Developer <random@developer.example.org> > OK. I would take care of it. >> >> > drivers/block/cciss.c | 4 +-- >> > drivers/block/cpqarray.c | 72 >> +++++++++++++++------------------------------- >> > drivers/block/loop.c | 4 +-- >> > 3 files changed, 25 insertions(+), 55 deletions(-) >> >> Your diffstat should have indicated to you that this should be split up >> better. Please (re-)read SubmittingPatches. *One* logical change per >> patch, most importantly. > OK. I would resubmit. >> > >> > diff --git a/drivers/block/cciss.c b/drivers/block/cciss.c >> > index 2cd3391..a800a69 100644 >> > --- a/drivers/block/cciss.c >> > +++ b/drivers/block/cciss.c >> > @@ -900,7 +900,7 @@ #if 0 /* 'buf_size' >> member is 16-bits >> > return -EINVAL; >> > #endif >> > if (iocommand.buf_size > 0) { >> > - buff = kmalloc(iocommand.buf_size, >> GFP_KERNEL); >> > + buff = kzalloc(iocommand.buf_size, >> GFP_KERNEL); >> > if (buff == NULL) >> > return -EFAULT; >> > } >> > @@ -911,8 +911,6 @@ #endif >> > kfree(buff); >> > return -EFAULT; >> > } >> > - } else { >> > - memset(buff, 0, iocommand.buf_size); >> > } >> > if ((c = cmd_alloc(host, 0)) == NULL) { >> > kfree(buff); >> >> This changes performance potentially, no? The memset before was >> conditional upon (iocommand.Request.Type.Direction == XFER_WRITE) and >> now the memory will always be zero'd. > Yes, but not the functionality. > if (iocommand.buf_size > 0), code allocates using kmalloc. if > direction is XFER_WRITE, it does a copy_from_user(), and free()s the > allocated buffer, not really caring what data came in from userspace. > Else, it does memset(). So I could safely replace the kmalloc() with > kzalloc() without compromising functionality.
Ok, this is something like I need 10 bytes of memory, so I request two memory pages for reserved use. It works, but it kills performance.
Why you zero memory that is not needed to be zeroed?
regards, -- http://www.fi.muni.cz/~xslaby/ Jiri Slaby faculty of informatics, masaryk university, brno, cz e-mail: jirislaby gmail com, gpg pubkey fingerprint: B674 9967 0407 CE62 ACC8 22A0 32CC 55C3 39D4 7A7E - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |