Dmitry Torokhov <dmitry.torokhov@xxxxxxxxx> wrote: >Hi Yuanhan, > >On Tue, Jan 08, 2013 at 10:57:53PM +0800, Yuanhan Liu wrote: >> The current kfifo API take the kfifo size as input, while it rounds >> _down_ the size to power of 2 at __kfifo_alloc. This may introduce >> potential issue. >> >> Take the code at drivers/hid/hid-logitech-dj.c as example: >> >> if (kfifo_alloc(&djrcv_dev->notif_fifo, >> DJ_MAX_NUMBER_NOTIFICATIONS * sizeof(struct >dj_report), >> GFP_KERNEL)) { >> >> Where, DJ_MAX_NUMBER_NOTIFICATIONS is 8, and sizeo of(struct >dj_report) >> is 15. >> >> Which means it wants to allocate a kfifo buffer which can store 8 >> dj_report entries at once. The expected kfifo buffer size would be >> 8 * 15 = 120 then. While, in the end, __kfifo_alloc will turn the >> size to rounddown_power_of_2(120) = 64, and then allocate a buf >> with 64 bytes, which I don't think this is the original author want. >> >> With the new log API, we can do like following: >> >> int kfifo_size_order = order_base_2(DJ_MAX_NUMBER_NOTIFICATIONS * >> sizeof(struct dj_report)); >> >> if (kfifo_alloc(&djrcv_dev->notif_fifo, kfifo_size_order, >GFP_KERNEL)) { >> >> This make sure we will allocate enough kfifo buffer for holding >> DJ_MAX_NUMBER_NOTIFICATIONS dj_report entries. > >Why don't you simply change __kfifo_alloc to round the allocation up >instead of down? > >Thanks. > >-- >Dmitry >-- >To unsubscribe from this list: send the line "unsubscribe linux-media" >in >the body of a message to majordomo@xxxxxxxxxxxxxxx >More majordomo info at http://vger.kernel.org/majordomo-info.html Hi Dmitry, I agree. I don't see the benefit in pushing up the change to a kfifo internal decision/problem to many different places in the kernel. Regards, Andy -- To unsubscribe from this list: send the line "unsubscribe linux-iio" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html