[PATCH 0/5] kfifo cleanup and log based kfifo API

[Yuanhan Liu <yuanhan.liu@linux.intel.com>]

The current kfifo API take the kfifo size as input, while it rounds
_down_ the size to power of 2 at __kfifo_alloc/init. This may introduce
several potential issues.

First, the kfifo size is not what we want. Say, if we want to
allocate a kfifo with size of 127 elements. Then in the end, we can
only hold 64 elements. Here is a similar example at
drivers/hid/hid-logitech-dj.c:

	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 something 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.


Second, kfifo_init used a pre-allocated buffer, and the actual
buffer size is determined in caller side, while the actual kfifo
buffer size is maintained in kfifo_init, which will be rounded down
power of 2 without notifying to caller. Take the example of code at
drivers/scsi/libiscsi.c:
        /* If the user passed an items pointer, he wants a copy of
         * the array. */
        if (items)
                num_arrays++;
        q->pool = kzalloc(num_arrays * max * sizeof(void*), GFP_KERNEL);
        if (q->pool == NULL)
                return -ENOMEM;

        kfifo_init(&q->queue, (void*)q->pool, max * sizeof(void*));

        for (i = 0; i < max; i++) {
                q->pool[i] = kzalloc(item_size, GFP_KERNEL);
                if (q->pool[i] == NULL) {
                        q->max = i;
                        goto enomem;
                }
                kfifo_in(&q->queue, (void*)&q->pool[i], sizeof(void*));
        }

Assume max is not power of 2, this piece of code will not work! As
it assumes the kfifo will hold max elements, but actually it can
only hold rounddown_power_of_2(max).

My proposal is to replace kfifo_init with kfifo_alloc, where it
allocate buffer and maintain fifo size inside kfifo. Then we can
remove buggy kfifo_init.

After we change the kfifo_alloc to be a log API, all above issue
will be gone as said by Stefani.

Here is the patch summary:
patch 1 is a cleanup patch to cleanup the unneeded type check.
patch 2 and 3 turn the only 2 users of kfifo_init to kfifo_alloc
patch 4 remove kfifo_init
patch 5 log based kfifo API


Thanks!

	--yliu

Cc: Stefani Seibold <stefani@seibold.net>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: James E.J. Bottomley <JBottomley@parallels.com>
Cc: iscsi list <open-iscsi@googlegroups.com>
Cc: Mike Christie <michaelc@cs.wisc.edu>
---
Yuanhan Liu (5):
  kfifo: remove unnecessary type check
  libsrp: replace kfifo_init with kfifo_alloc
  libiscsi: replace kfifo_init with kfifo_alloc
  kfifo: remove kfifo_init
  kfifo: log based kfifo API

 arch/arm/plat-omap/Kconfig                  |    2 +-
 arch/arm/plat-omap/mailbox.c                |    6 +-
 arch/powerpc/sysdev/fsl_rmu.c               |    2 +-
 drivers/char/sonypi.c                       |    9 ++-
 drivers/hid/hid-logitech-dj.c               |    7 +-
 drivers/iio/industrialio-event.c            |    2 +-
 drivers/iio/kfifo_buf.c                     |    3 +-
 drivers/infiniband/hw/cxgb3/cxio_resource.c |    8 ++-
 drivers/media/i2c/cx25840/cx25840-ir.c      |    9 ++-
 drivers/media/pci/cx23885/cx23888-ir.c      |    9 ++-
 drivers/media/pci/meye/meye.c               |    6 +-
 drivers/media/pci/meye/meye.h               |    2 +
 drivers/media/rc/ir-raw.c                   |    7 +-
 drivers/memstick/host/r592.h                |    2 +-
 drivers/mmc/card/sdio_uart.c                |    4 +-
 drivers/mtd/sm_ftl.c                        |    5 +-
 drivers/net/wireless/libertas/main.c        |    4 +-
 drivers/net/wireless/rt2x00/rt2x00dev.c     |    5 +-
 drivers/pci/pcie/aer/aerdrv_core.c          |    3 +-
 drivers/platform/x86/fujitsu-laptop.c       |    5 +-
 drivers/platform/x86/sony-laptop.c          |    6 +-
 drivers/rapidio/devices/tsi721.c            |    5 +-
 drivers/scsi/libiscsi.c                     |   59 ++++++++--------
 drivers/scsi/libiscsi_tcp.c                 |    6 +-
 drivers/scsi/libsrp.c                       |   22 +++---
 drivers/staging/omapdrm/omap_plane.c        |    5 +-
 drivers/tty/n_gsm.c                         |    4 +-
 drivers/tty/nozomi.c                        |    5 +-
 drivers/tty/serial/ifx6x60.c                |    2 +-
 drivers/tty/serial/ifx6x60.h                |    3 +-
 drivers/tty/serial/kgdb_nmi.c               |    7 +-
 drivers/usb/host/fhci.h                     |    4 +-
 drivers/usb/serial/cypress_m8.c             |    4 +-
 drivers/usb/serial/io_ti.c                  |    4 +-
 drivers/usb/serial/ti_usb_3410_5052.c       |    7 +-
 drivers/usb/serial/usb-serial.c             |    2 +-
 include/linux/kfifo.h                       |   98 +++++++--------------------
 include/linux/rio.h                         |    1 +
 include/media/lirc_dev.h                    |    4 +-
 include/scsi/libiscsi.h                     |    4 +-
 include/scsi/libsrp.h                       |    1 -
 kernel/kfifo.c                              |   32 +--------
 mm/memory-failure.c                         |    3 +-
 net/dccp/probe.c                            |    6 +-
 net/sctp/probe.c                            |    6 +-
 samples/kfifo/bytestream-example.c          |    8 +-
 samples/kfifo/dma-example.c                 |    5 +-
 samples/kfifo/inttype-example.c             |    7 +-
 samples/kfifo/record-example.c              |    6 +-
 49 files changed, 195 insertions(+), 231 deletions(-)

-- 
1.7.7.6