All of lore.kernel.org
 help / color / mirror / Atom feed
* ->poll() instances shouldn't be indefinitely blocking
@ 2015-11-27  5:00 Al Viro
  2015-11-27 15:18 ` Mauro Carvalho Chehab
  0 siblings, 1 reply; 12+ messages in thread
From: Al Viro @ 2015-11-27  5:00 UTC (permalink / raw)
  To: Mauro Carvalho Chehab; +Cc: Linus Torvalds, linux-kernel, linux-fsdevel

Take a look at this:
static unsigned int gsc_m2m_poll(struct file *file,
                                        struct poll_table_struct *wait)
{
        struct gsc_ctx *ctx = fh_to_ctx(file->private_data);
        struct gsc_dev *gsc = ctx->gsc_dev;
        int ret;

        if (mutex_lock_interruptible(&gsc->lock))
                return -ERESTARTSYS;

        ret = v4l2_m2m_poll(file, ctx->m2m_ctx, wait);
        mutex_unlock(&gsc->lock);

        return ret;
}

a) ->poll() should not return -E...; callers expect just a bitmap of
POLL... values.

b) sure, it's nice that if this thing hangs, we'll be able to kill it.
However, if one's ->poll() can hang indefinitely, it means bad things
for poll(2), select(2), etc. semantics.  What the hell had been intended
there?

c) a bunch of v4l2_m2m_poll() callers are also taking some kind of
mutex; AFAICS, all of those appear bogus (the rest of them do not
play wiht ERESTARTSYS, just plain mutex_lock() for those).

What's going on there?

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: ->poll() instances shouldn't be indefinitely blocking
  2015-11-27  5:00 ->poll() instances shouldn't be indefinitely blocking Al Viro
@ 2015-11-27 15:18 ` Mauro Carvalho Chehab
  2015-11-27 17:49   ` Linus Torvalds
  0 siblings, 1 reply; 12+ messages in thread
From: Mauro Carvalho Chehab @ 2015-11-27 15:18 UTC (permalink / raw)
  To: Al Viro
  Cc: Linus Torvalds, linux-kernel, linux-fsdevel, linux-media,
	Kukjin Kim, Krzysztof Kozlowski, Hans Verkuil

Em Fri, 27 Nov 2015 05:00:26 +0000
Al Viro <viro@ZenIV.linux.org.uk> escreveu:

> Take a look at this:
> static unsigned int gsc_m2m_poll(struct file *file,
>                                         struct poll_table_struct *wait)
> {
>         struct gsc_ctx *ctx = fh_to_ctx(file->private_data);
>         struct gsc_dev *gsc = ctx->gsc_dev;
>         int ret;
> 
>         if (mutex_lock_interruptible(&gsc->lock))
>                 return -ERESTARTSYS;
> 
>         ret = v4l2_m2m_poll(file, ctx->m2m_ctx, wait);
>         mutex_unlock(&gsc->lock);
> 
>         return ret;
> }
> 
> a) ->poll() should not return -E...; callers expect just a bitmap of
> POLL... values.

Yeah. We fixed issues like that on other drivers along the time. I guess
this is a some bad code that people just cut-and-paste from legacy drivers
without looking into it.

The same kind of crap were found (and fixed) on other drivers, like
the fix on this changeset: 45053edc05 ('[media] saa7164: fix poll bugs').

> b) sure, it's nice that if this thing hangs, we'll be able to kill it.
> However, if one's ->poll() can hang indefinitely, it means bad things
> for poll(2), select(2), etc. semantics.  What the hell had been intended
> there?

I guess there was no special intent. It is just a bad driver code that
was replicated from other drivers.

> c) a bunch of v4l2_m2m_poll() callers are also taking some kind of
> mutex; AFAICS, all of those appear bogus (the rest of them do not
> play wiht ERESTARTSYS, just plain mutex_lock() for those).
> 
> What's going on there?

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: ->poll() instances shouldn't be indefinitely blocking
  2015-11-27 15:18 ` Mauro Carvalho Chehab
@ 2015-11-27 17:49   ` Linus Torvalds
  2015-11-30  3:04     ` Al Viro
  0 siblings, 1 reply; 12+ messages in thread
From: Linus Torvalds @ 2015-11-27 17:49 UTC (permalink / raw)
  To: Mauro Carvalho Chehab
  Cc: Al Viro, Linux Kernel Mailing List, linux-fsdevel,
	Linux Media Mailing List, Kukjin Kim, Krzysztof Kozlowski,
	Hans Verkuil

On Fri, Nov 27, 2015 at 7:18 AM, Mauro Carvalho Chehab
<mchehab@osg.samsung.com> wrote:
> Al Viro <viro@ZenIV.linux.org.uk> escreveu:
>
>> Take a look at this:
>> static unsigned int gsc_m2m_poll(struct file *file,
>>                                         struct poll_table_struct *wait)
>> {
>>         struct gsc_ctx *ctx = fh_to_ctx(file->private_data);
>>         struct gsc_dev *gsc = ctx->gsc_dev;
>>         int ret;
>>
>>         if (mutex_lock_interruptible(&gsc->lock))
>>                 return -ERESTARTSYS;
>>
>>         ret = v4l2_m2m_poll(file, ctx->m2m_ctx, wait);
>>         mutex_unlock(&gsc->lock);
>>
>>         return ret;
>> }
>>
>> a) ->poll() should not return -E...; callers expect just a bitmap of
>> POLL... values.
>
> Yeah. We fixed issues like that on other drivers along the time. I guess
> this is a some bad code that people just cut-and-paste from legacy drivers
> without looking into it.

Actually, while returning -ERESTARTSYS is bogus, returning _zero_
would not be. The top-level poll() code will happily notice the
signal, and return -EINTR like poll should (unless something else is
pending, in which case it will return zero and the bits set for that
something else).

So having a driver with a ->poll() function that does that kind of
conditional locking is not wrong per se. It's just he return value
that is crap.

I also do wonder if we might not make the generic code a bit more
robust wrt things like this. The bitmask we use is only about the low
bits, so we *could* certainly allow the driver poll() functions to
return errors - possibly just ignoring them. Or perhaps have a
WARN_ON_OCNE() to find them.

Al, what do you think? The whole "generic code should be robust wrt
drivers making silly mistakes" just sounds like a good idea. Finding
these things through code inspection is all well and good, but having
a nice warning report from users might be even better.

                Linus

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: ->poll() instances shouldn't be indefinitely blocking
  2015-11-27 17:49   ` Linus Torvalds
@ 2015-11-30  3:04     ` Al Viro
  2015-12-04  6:38       ` more POLL... fun Al Viro
  0 siblings, 1 reply; 12+ messages in thread
From: Al Viro @ 2015-11-30  3:04 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: Mauro Carvalho Chehab, Linux Kernel Mailing List, linux-fsdevel,
	Linux Media Mailing List, Kukjin Kim, Krzysztof Kozlowski,
	Hans Verkuil

On Fri, Nov 27, 2015 at 09:49:11AM -0800, Linus Torvalds wrote:

> Al, what do you think? The whole "generic code should be robust wrt
> drivers making silly mistakes" just sounds like a good idea. Finding
> these things through code inspection is all well and good, but having
> a nice warning report from users might be even better.

FWIW, sparse catches them just fine here.  I don't mind using the MSB
as a canary, but AFAICS that code tends to be rarely hit, so this
kind of runtime warning isn't going to be all that useful.

The current list is:

drivers/media/platform/exynos-gsc/gsc-m2m.c:707:24:    expected restricted __poll_t
drivers/media/platform/exynos-gsc/gsc-m2m.c:707:24:    got int
drivers/media/platform/exynos-gsc/gsc-m2m.c:707:24: warning: incorrect type in return expression (different base types)
--- this one
drivers/media/platform/s3c-camif/camif-capture.c:615:21:    expected restricted __poll_t [usertype] ret
drivers/media/platform/s3c-camif/camif-capture.c:615:21:    got int
drivers/media/platform/s3c-camif/camif-capture.c:615:21: warning: incorrect type in assignment (different base types)
        mutex_lock(&camif->lock);
        if (vp->owner && vp->owner != file->private_data)
                ret = -EBUSY;
        else   
                ret = vb2_poll(&vp->vb_queue, file, wait);
--- not nice
drivers/media/radio/radio-wl1273.c:1092:24:    expected restricted __poll_t
drivers/media/radio/radio-wl1273.c:1092:24:    got int
drivers/media/radio/radio-wl1273.c:1092:24: warning: incorrect type in return expression (different base types)
        struct wl1273_device *radio = video_get_drvdata(video_devdata(file));
        struct wl1273_core *core = radio->core;

        if (radio->owner && radio->owner != file)
                return -EBUSY;

        radio->owner = file;
--- obviously racy, not to mention anything else (that's the very beginning of
->poll() instance)
drivers/staging/rdma/hfi1/diag.c:798:24:    expected restricted __poll_t
drivers/staging/rdma/hfi1/diag.c:798:24:    got int
drivers/staging/rdma/hfi1/diag.c:798:24: warning: incorrect type in return expression (different base types)
        dd = hfi1_dd_from_sc_inode(fp->f_inode);
        if (dd == NULL)
                return -ENODEV;
--- hadn't looked yet
drivers/misc/mei/main.c:668:24:    expected int
drivers/misc/mei/main.c:668:24:    got restricted __poll_t [usertype] <noident>
drivers/misc/mei/main.c:668:24: warning: incorrect type in return expression (different base types)
--- this one is other way round - it's ->fasync() returning POLLERR; the
author told of that one, says it's a cut'n'paste bug.  Hopefully will get
fixed...
drivers/tty/n_r3964.c:1241:24:    expected restricted __poll_t [assigned] [usertype] result
drivers/tty/n_r3964.c:1241:24:    got int
drivers/tty/n_r3964.c:1241:24: warning: incorrect type in assignment (different base types)
        pClient = findClient(pInfo, task_pid(current));
        if (pClient) {
...
        } else {
                result = -EINVAL;
--- maybe it's just me, but this looks utterly bogus.  task_pid(current)?
Really?
drivers/uio/uio.c:497:24:    expected restricted __poll_t
drivers/uio/uio.c:497:24:    got int
drivers/uio/uio.c:497:24: warning: incorrect type in return expression (different base types)
        if (!idev->info->irq)
                return -EIO;
--- bogosity aside, I really don't like the look of the lifetime
rules for the thing idev->info points to in that one.  Looks like
unregistration *can* happen while the sucker's open, and it won't
wait for anything to run down.
kernel/time/posix-clock.c:75:24:    expected restricted __poll_t
kernel/time/posix-clock.c:75:24:    got int
--- that one does deal with unregistration, but (a) returns a bogus
value and (b) does down_read(&clk->rwsem), which might be not nice
in ->poll() instance.
kernel/trace/ring_buffer.c:640:32:    expected restricted __poll_t
kernel/trace/ring_buffer.c:640:32:    got int
kernel/trace/ring_buffer.c:640:32: warning: incorrect type in return expression (different base types)
--- might be racy, in any case the return value is bogus
sound/core/compress_offload.c:381:24:    expected restricted __poll_t
sound/core/compress_offload.c:381:24:    got int
sound/core/compress_offload.c:381:24: warning: incorrect type in return expression (different base types)
sound/core/compress_offload.c:384:24:    expected restricted __poll_t
sound/core/compress_offload.c:384:24:    got int
sound/core/compress_offload.c:384:24: warning: incorrect type in return expression (different base types)
--- a couple of "what do we return after struct file being
stomped upon, because it can't happen in any other way"
sound/core/compress_offload.c:388:24:    expected restricted __poll_t [usertype] retval
sound/core/compress_offload.c:388:24:    got int
sound/core/compress_offload.c:388:24: warning: incorrect type in assignment (different base types)
sound/core/pcm_native.c:3152:24:    expected restricted __poll_t
sound/core/pcm_native.c:3152:24:    got int
sound/core/pcm_native.c:3152:24: warning: incorrect type in return expression (different base types)
sound/core/pcm_native.c:3191:24:    expected restricted __poll_t
sound/core/pcm_native.c:3191:24:    got int
sound/core/pcm_native.c:3191:24: warning: incorrect type in return expression (different base types)
--- not sure, hadn't looked in detail.
sound/core/seq/oss/seq_oss.c:203:24:    expected restricted __poll_t
sound/core/seq/oss/seq_oss.c:203:24:    got int
sound/core/seq/oss/seq_oss.c:203:24: warning: incorrect type in return expression (different base types)
--- impossible to hit without struct file being corrupted
sound/core/seq/seq_clientmgr.c:1102:24:    expected restricted __poll_t
sound/core/seq/seq_clientmgr.c:1102:24:    got int
sound/core/seq/seq_clientmgr.c:1102:24: warning: incorrect type in return expression (different base types)
--- ditto
drivers/gpu/vga/vgaarb.c:1169:24: warning: incorrect type in return expression (different base types)
drivers/gpu/vga/vgaarb.c:1169:24:    expected restricted __poll_t
drivers/gpu/vga/vgaarb.c:1169:24:    got int
--- ditto

That's on amd64/allmodconfig build with the annotations I'd done for
->poll().  They also have caught a couple of dubious places in net/9p,
still looking in there.  FWIW, the typical impact of annotations on a
driver is something like
@@ -2155,11 +2155,11 @@ pmu_write(struct file *file, const char __user *buf,
        return 0;
 }
 
-static unsigned int
+static __poll_t
 pmu_fpoll(struct file *filp, poll_table *wait)
 {
        struct pmu_private *pp = filp->private_data;
-       unsigned int mask = 0;
+       __poll_t mask = 0;
        unsigned long flags;
        
        if (pp == 0)

IOW, not really intrusive.  make C=2 CF="-D__CHECK_ENDIAN -D__CHECK_POLL__"
right now, might as well make poll annotations trigger on __CHECK_ENDIAN__
alone - there's not much noise left.  Again, I have no problem with
adding "if the MSB is set, it's probably a broken driver returning -E...
instead of the valid bitmap, let's warn about it", but IMO it's better
dealt with by sparse at build time; bogus values are not returned on
common paths, so the runtime test coverage won't be good.

See vfs.git#work.poll-annotations for the current state of that queue.  The
last commit needs to be split and folded back into the relevant commits
before - the queue was originally done back in March and last week I'd
ported it to current mainline, the last commit being the annotations
in the code that had been added since then.

The current summary follows:
Shortlog:
Al Viro (20):
      switch poll.h to generic-y where possible
      annotate POLL... constants and ->poll() return value
      annotate poll_table_struct->_key and poll_table_entry->key
      annotate wake_..._poll() last argument
      fs: annotate ->poll() instances
      annotate proto_ops ->poll()
      annotate proto_ops ->poll() instances
      net: annotate file_operations ->poll() instances
      kernel: annotate ->poll() instances
      annotate posix clock
      annotate security/
      annotate mm/
      annotate ipc/
      annotate drivers/media and its users
      annotate sound/
      annotate tty
      annotate assorted drivers
      VFS annotations
      annotations around wakeup callbacks
      missing ->poll() annotations [splitme]

Diffstat:
 arch/alpha/include/asm/Kbuild                      |  1 +
 arch/alpha/include/uapi/asm/poll.h                 |  1 -
 arch/blackfin/include/uapi/asm/poll.h              |  4 +--
 arch/cris/arch-v10/drivers/gpio.c                  |  6 ++--
 arch/cris/arch-v10/drivers/sync_serial.c           |  8 ++---
 arch/cris/arch-v32/drivers/sync_serial.c           |  8 ++---
 arch/frv/include/uapi/asm/poll.h                   |  2 +-
 arch/ia64/include/asm/Kbuild                       |  1 +
 arch/ia64/include/uapi/asm/poll.h                  |  1 -
 arch/ia64/kernel/perfmon.c                         |  4 +--
 arch/m32r/include/asm/Kbuild                       |  1 +
 arch/m32r/include/uapi/asm/poll.h                  |  1 -
 arch/m68k/include/uapi/asm/poll.h                  |  2 +-
 arch/microblaze/include/asm/Kbuild                 |  1 +
 arch/microblaze/include/uapi/asm/poll.h            |  1 -
 arch/mips/include/uapi/asm/poll.h                  |  2 +-
 arch/mips/kernel/rtlx.c                            |  4 +--
 arch/mn10300/include/asm/Kbuild                    |  1 +
 arch/mn10300/include/uapi/asm/poll.h               |  1 -
 arch/powerpc/include/asm/Kbuild                    |  1 +
 arch/powerpc/include/uapi/asm/poll.h               |  1 -
 arch/powerpc/kernel/rtasd.c                        |  2 +-
 arch/powerpc/platforms/cell/spufs/file.c           | 16 +++++-----
 arch/powerpc/platforms/powernv/opal-prd.c          |  2 +-
 arch/s390/include/asm/Kbuild                       |  1 +
 arch/s390/include/uapi/asm/poll.h                  |  1 -
 arch/score/include/asm/Kbuild                      |  1 +
 arch/score/include/uapi/asm/poll.h                 |  6 ----
 arch/sparc/include/uapi/asm/poll.h                 |  8 ++---
 arch/um/drivers/hostaudio_kern.c                   |  4 +--
 arch/um/include/asm/Kbuild                         |  1 +
 arch/x86/include/asm/Kbuild                        |  1 +
 arch/x86/include/uapi/asm/poll.h                   |  1 -
 arch/x86/kernel/apm_32.c                           |  2 +-
 arch/x86/kernel/cpu/mcheck/mce.c                   |  2 +-
 arch/xtensa/include/uapi/asm/poll.h                |  4 +--
 block/bsg.c                                        |  4 +--
 crypto/algif_aead.c                                |  4 +--
 crypto/algif_skcipher.c                            |  6 ++--
 drivers/android/binder.c                           |  2 +-
 drivers/bluetooth/hci_ldisc.c                      |  2 +-
 drivers/bluetooth/hci_vhci.c                       |  2 +-
 drivers/char/apm-emulation.c                       |  2 +-
 drivers/char/dtlk.c                                |  6 ++--
 drivers/char/genrtc.c                              |  2 +-
 drivers/char/hpet.c                                |  2 +-
 drivers/char/ipmi/ipmi_devintf.c                   |  4 +--
 drivers/char/ipmi/ipmi_watchdog.c                  |  4 +--
 drivers/char/pcmcia/cm4040_cs.c                    |  4 +--
 drivers/char/ppdev.c                               |  4 +--
 drivers/char/random.c                              |  4 +--
 drivers/char/rtc.c                                 |  4 +--
 drivers/char/snsc.c                                |  4 +--
 drivers/char/sonypi.c                              |  2 +-
 drivers/char/virtio_console.c                      |  4 +--
 drivers/char/xillybus/xillybus_core.c              |  4 +--
 drivers/dma-buf/dma-buf.c                          |  6 ++--
 drivers/firewire/core-cdev.c                       |  4 +--
 drivers/firewire/nosy.c                            |  4 +--
 drivers/gpu/drm/drm_fops.c                         |  4 +--
 drivers/gpu/drm/vmwgfx/vmwgfx_drv.h                |  2 +-
 drivers/gpu/drm/vmwgfx/vmwgfx_ioctl.c              |  2 +-
 drivers/gpu/vga/vgaarb.c                           |  2 +-
 drivers/hid/hid-debug.c                            |  2 +-
 drivers/hid/hid-roccat.c                           |  2 +-
 drivers/hid/hid-sensor-custom.c                    |  4 +--
 drivers/hid/hidraw.c                               |  2 +-
 drivers/hid/uhid.c                                 |  2 +-
 drivers/hid/usbhid/hiddev.c                        |  2 +-
 drivers/hsi/clients/cmt_speech.c                   |  4 +--
 drivers/hv/hv_utils_transport.c                    |  2 +-
 drivers/iio/iio_core.h                             |  2 +-
 drivers/iio/industrialio-buffer.c                  |  2 +-
 drivers/iio/industrialio-event.c                   |  4 +--
 drivers/infiniband/core/ucm.c                      |  4 +--
 drivers/infiniband/core/ucma.c                     |  4 +--
 drivers/infiniband/core/user_mad.c                 |  4 +--
 drivers/infiniband/core/uverbs_main.c              |  4 +--
 drivers/infiniband/hw/qib/qib_file_ops.c           | 14 ++++-----
 drivers/input/evdev.c                              |  4 +--
 drivers/input/input.c                              |  2 +-
 drivers/input/joydev.c                             |  2 +-
 drivers/input/misc/hp_sdc_rtc.c                    |  4 +--
 drivers/input/misc/uinput.c                        |  2 +-
 drivers/input/mousedev.c                           |  4 +--
 drivers/input/serio/serio_raw.c                    |  4 +--
 drivers/input/serio/userio.c                       |  2 +-
 drivers/isdn/capi/capi.c                           |  4 +--
 drivers/isdn/divert/divert_procfs.c                |  4 +--
 drivers/isdn/hardware/eicon/divamnt.c              |  4 +--
 drivers/isdn/hardware/eicon/divasi.c               |  4 +--
 drivers/isdn/hardware/eicon/divasmain.c            |  2 +-
 drivers/isdn/hardware/eicon/divasproc.c            |  2 +-
 drivers/isdn/hysdn/hysdn_proclog.c                 |  4 +--
 drivers/isdn/i4l/isdn_common.c                     |  4 +--
 drivers/isdn/i4l/isdn_ppp.c                        |  4 +--
 drivers/isdn/i4l/isdn_ppp.h                        |  2 +-
 drivers/isdn/mISDN/timerdev.c                      |  4 +--
 drivers/macintosh/smu.c                            |  4 +--
 drivers/macintosh/via-pmu.c                        |  4 +--
 drivers/md/md.c                                    |  4 +--
 drivers/media/common/saa7146/saa7146_fops.c        |  8 ++---
 drivers/media/common/siano/smsdvb-debugfs.c        |  7 ++---
 drivers/media/dvb-core/dmxdev.c                    |  8 ++---
 drivers/media/dvb-core/dvb_ca_en50221.c            |  4 +--
 drivers/media/dvb-core/dvb_frontend.c              |  2 +-
 drivers/media/firewire/firedtv-ci.c                |  2 +-
 drivers/media/media-devnode.c                      |  2 +-
 drivers/media/pci/bt8xx/bttv-driver.c              | 12 ++++----
 drivers/media/pci/cx18/cx18-fileops.c              |  8 ++---
 drivers/media/pci/cx18/cx18-fileops.h              |  2 +-
 drivers/media/pci/ddbridge/ddbridge-core.c         |  4 +--
 drivers/media/pci/ivtv/ivtv-fileops.c              | 10 +++---
 drivers/media/pci/ivtv/ivtv-fileops.h              |  4 +--
 drivers/media/pci/meye/meye.c                      |  4 +--
 drivers/media/pci/saa7134/saa7134-video.c          |  4 +--
 drivers/media/pci/saa7164/saa7164-encoder.c        |  6 ++--
 drivers/media/pci/saa7164/saa7164-vbi.c            |  4 +--
 drivers/media/pci/ttpci/av7110_av.c                |  8 ++---
 drivers/media/pci/ttpci/av7110_ca.c                |  4 +--
 drivers/media/pci/zoran/zoran_driver.c             |  4 +--
 drivers/media/platform/davinci/vpfe_capture.c      |  2 +-
 drivers/media/platform/exynos-gsc/gsc-m2m.c        |  4 +--
 drivers/media/platform/fsl-viu.c                   |  6 ++--
 drivers/media/platform/m2m-deinterlace.c           |  4 +--
 drivers/media/platform/mx2_emmaprp.c               |  4 +--
 drivers/media/platform/omap/omap_vout.c            |  2 +-
 drivers/media/platform/omap3isp/ispvideo.c         |  4 +--
 drivers/media/platform/s3c-camif/camif-capture.c   |  4 +--
 drivers/media/platform/s5p-mfc/s5p_mfc.c           |  4 +--
 drivers/media/platform/s5p-tv/mixer_video.c        |  4 +--
 drivers/media/platform/sh_veu.c                    |  2 +-
 drivers/media/platform/soc_camera/atmel-isi.c      |  2 +-
 drivers/media/platform/soc_camera/mx2_camera.c     |  2 +-
 drivers/media/platform/soc_camera/mx3_camera.c     |  2 +-
 drivers/media/platform/soc_camera/omap1_camera.c   |  2 +-
 drivers/media/platform/soc_camera/pxa_camera.c     |  2 +-
 drivers/media/platform/soc_camera/rcar_vin.c       |  2 +-
 .../platform/soc_camera/sh_mobile_ceu_camera.c     |  2 +-
 drivers/media/platform/soc_camera/soc_camera.c     |  4 +--
 drivers/media/platform/timblogiw.c                 |  2 +-
 drivers/media/platform/via-camera.c                |  2 +-
 drivers/media/platform/vivid/vivid-core.c          |  2 +-
 drivers/media/platform/vivid/vivid-radio-rx.c      |  2 +-
 drivers/media/platform/vivid/vivid-radio-rx.h      |  2 +-
 drivers/media/platform/vivid/vivid-radio-tx.c      |  2 +-
 drivers/media/platform/vivid/vivid-radio-tx.h      |  2 +-
 drivers/media/radio/radio-cadet.c                  |  6 ++--
 drivers/media/radio/radio-si476x.c                 |  6 ++--
 drivers/media/radio/radio-wl1273.c                 |  2 +-
 drivers/media/radio/si470x/radio-si470x-common.c   |  6 ++--
 drivers/media/radio/wl128x/fmdrv_v4l2.c            |  2 +-
 drivers/media/rc/lirc_dev.c                        |  4 +--
 drivers/media/usb/cpia2/cpia2.h                    |  4 +--
 drivers/media/usb/cpia2/cpia2_core.c               |  4 +--
 drivers/media/usb/cpia2/cpia2_v4l.c                |  4 +--
 drivers/media/usb/cx231xx/cx231xx-417.c            |  6 ++--
 drivers/media/usb/cx231xx/cx231xx-video.c          |  6 ++--
 drivers/media/usb/gspca/gspca.c                    |  6 ++--
 drivers/media/usb/hdpvr/hdpvr-video.c              |  6 ++--
 drivers/media/usb/pvrusb2/pvrusb2-v4l2.c           |  4 +--
 drivers/media/usb/stkwebcam/stk-webcam.c           |  4 +--
 drivers/media/usb/tm6000/tm6000-video.c            | 10 +++---
 drivers/media/usb/uvc/uvc_queue.c                  |  4 +--
 drivers/media/usb/uvc/uvc_v4l2.c                   |  2 +-
 drivers/media/usb/uvc/uvcvideo.h                   |  2 +-
 drivers/media/usb/zr364xx/zr364xx.c                |  4 +--
 drivers/media/v4l2-core/v4l2-ctrls.c               |  2 +-
 drivers/media/v4l2-core/v4l2-dev.c                 |  4 +--
 drivers/media/v4l2-core/v4l2-mem2mem.c             | 10 +++---
 drivers/media/v4l2-core/v4l2-subdev.c              |  2 +-
 drivers/media/v4l2-core/videobuf-core.c            | 10 +++---
 drivers/media/v4l2-core/videobuf2-v4l2.c           | 10 +++---
 drivers/misc/cxl/api.c                             |  2 +-
 drivers/misc/cxl/cxl.h                             |  2 +-
 drivers/misc/cxl/file.c                            |  4 +--
 drivers/misc/hpilo.c                               |  2 +-
 drivers/misc/lis3lv02d/lis3lv02d.c                 |  2 +-
 drivers/misc/mei/amthif.c                          |  4 +--
 drivers/misc/mei/main.c                            |  6 ++--
 drivers/misc/mei/mei_dev.h                         |  2 +-
 drivers/misc/mic/host/mic_fops.c                   |  4 +--
 drivers/misc/mic/host/mic_fops.h                   |  2 +-
 drivers/misc/mic/scif/scif_api.c                   |  7 +++--
 drivers/misc/mic/scif/scif_epd.h                   |  2 +-
 drivers/misc/mic/scif/scif_fd.c                    |  2 +-
 drivers/misc/phantom.c                             |  4 +--
 drivers/misc/vmw_vmci/vmci_host.c                  |  4 +--
 drivers/net/macvtap.c                              |  4 +--
 drivers/net/ppp/ppp_async.c                        |  2 +-
 drivers/net/ppp/ppp_generic.c                      |  4 +--
 drivers/net/ppp/ppp_synctty.c                      |  2 +-
 drivers/net/tun.c                                  |  4 +--
 drivers/net/wan/cosa.c                             |  4 +--
 drivers/net/wireless/rt2x00/rt2x00debug.c          |  2 +-
 drivers/platform/goldfish/goldfish_pipe.c          |  4 +--
 drivers/platform/x86/sony-laptop.c                 |  2 +-
 drivers/pps/pps.c                                  |  2 +-
 drivers/ptp/ptp_chardev.c                          |  2 +-
 drivers/ptp/ptp_private.h                          |  2 +-
 drivers/rtc/rtc-dev.c                              |  2 +-
 drivers/s390/block/dasd_eer.c                      |  4 +--
 drivers/s390/char/monreader.c                      |  2 +-
 drivers/scsi/megaraid/megaraid_sas_base.c          |  4 +--
 drivers/scsi/mpt3sas/mpt3sas_ctl.c                 |  2 +-
 drivers/scsi/sg.c                                  |  4 +--
 drivers/staging/android/sync.c                     |  2 +-
 drivers/staging/comedi/comedi_fops.c               |  4 +--
 drivers/staging/comedi/drivers/serial2002.c        |  2 +-
 drivers/staging/media/bcm2048/radio-bcm2048.c      |  4 +--
 drivers/staging/media/davinci_vpfe/vpfe_video.c    |  2 +-
 drivers/staging/media/lirc/lirc_parallel.c         |  2 +-
 drivers/staging/media/lirc/lirc_sir.c              |  4 +--
 drivers/staging/media/lirc/lirc_zilog.c            |  4 +--
 drivers/staging/media/omap4iss/iss_video.c         |  2 +-
 drivers/staging/most/aim-cdev/cdev.c               |  4 +--
 drivers/staging/most/aim-v4l2/video.c              |  4 +--
 drivers/staging/rdma/hfi1/diag.c                   |  6 ++--
 drivers/staging/rdma/hfi1/file_ops.c               | 18 +++++------
 drivers/staging/rdma/ipath/ipath_file_ops.c        | 18 +++++------
 drivers/staging/speakup/speakup_soft.c             |  4 +--
 drivers/tty/n_gsm.c                                |  4 +--
 drivers/tty/n_hdlc.c                               |  6 ++--
 drivers/tty/n_r3964.c                              |  6 ++--
 drivers/tty/n_tty.c                                |  4 +--
 drivers/tty/tty_io.c                               |  8 ++---
 drivers/tty/vt/vc_screen.c                         |  4 +--
 drivers/uio/uio.c                                  |  2 +-
 drivers/usb/class/cdc-wdm.c                        |  4 +--
 drivers/usb/class/usblp.c                          |  4 +--
 drivers/usb/core/devices.c                         |  2 +-
 drivers/usb/core/devio.c                           |  4 +--
 drivers/usb/gadget/function/f_fs.c                 |  4 +--
 drivers/usb/gadget/function/f_hid.c                |  4 +--
 drivers/usb/gadget/function/f_printer.c            |  4 +--
 drivers/usb/gadget/function/uvc_queue.c            |  2 +-
 drivers/usb/gadget/function/uvc_queue.h            |  2 +-
 drivers/usb/gadget/function/uvc_v4l2.c             |  2 +-
 drivers/usb/gadget/legacy/inode.c                  |  4 +--
 drivers/usb/misc/iowarrior.c                       |  4 +--
 drivers/usb/misc/ldusb.c                           |  4 +--
 drivers/usb/misc/legousbtower.c                    |  6 ++--
 drivers/usb/mon/mon_bin.c                          |  4 +--
 drivers/vfio/virqfd.c                              |  4 +--
 drivers/vhost/vhost.c                              |  8 ++---
 drivers/vhost/vhost.h                              |  4 +--
 drivers/virt/fsl_hypervisor.c                      |  4 +--
 drivers/xen/evtchn.c                               |  4 +--
 drivers/xen/mcelog.c                               |  2 +-
 drivers/xen/xenbus/xenbus_dev_frontend.c           |  2 +-
 fs/cachefiles/daemon.c                             | 10 +++---
 fs/coda/psdev.c                                    |  4 +--
 fs/dlm/plock.c                                     |  4 +--
 fs/dlm/user.c                                      |  2 +-
 fs/ecryptfs/miscdev.c                              |  4 +--
 fs/eventfd.c                                       |  4 +--
 fs/eventpoll.c                                     |  6 ++--
 fs/fcntl.c                                         |  4 +--
 fs/fuse/dev.c                                      |  4 +--
 fs/fuse/file.c                                     |  2 +-
 fs/fuse/fuse_i.h                                   |  2 +-
 fs/kernfs/file.c                                   |  2 +-
 fs/notify/fanotify/fanotify_user.c                 |  4 +--
 fs/notify/inotify/inotify_user.c                   |  4 +--
 fs/ocfs2/dlmfs/dlmfs.c                             |  4 +--
 fs/pipe.c                                          |  4 +--
 fs/proc/inode.c                                    |  6 ++--
 fs/proc/kmsg.c                                     |  2 +-
 fs/proc/proc_sysctl.c                              |  4 +--
 fs/proc_namespace.c                                |  4 +--
 fs/select.c                                        | 23 ++++++--------
 fs/signalfd.c                                      |  4 +--
 fs/timerfd.c                                       |  4 +--
 fs/userfaultfd.c                                   |  4 +--
 include/drm/drmP.h                                 |  2 +-
 include/linux/dma-buf.h                            |  2 +-
 include/linux/fs.h                                 |  2 +-
 include/linux/net.h                                |  2 +-
 include/linux/poll.h                               | 10 +++---
 include/linux/posix-clock.h                        |  2 +-
 include/linux/ring_buffer.h                        |  2 +-
 include/linux/scif.h                               |  4 +--
 include/linux/skbuff.h                             |  4 +--
 include/linux/tty_ldisc.h                          |  2 +-
 include/linux/wait.h                               | 10 +++---
 include/media/lirc_dev.h                           |  2 +-
 include/media/media-devnode.h                      |  2 +-
 include/media/soc_camera.h                         |  2 +-
 include/media/v4l2-ctrls.h                         |  2 +-
 include/media/v4l2-dev.h                           |  2 +-
 include/media/v4l2-mem2mem.h                       |  4 +--
 include/media/videobuf-core.h                      |  2 +-
 include/media/videobuf2-v4l2.h                     |  4 +--
 include/net/bluetooth/bluetooth.h                  |  2 +-
 include/net/inet_connection_sock.h                 |  2 +-
 include/net/iucv/af_iucv.h                         |  4 +--
 include/net/sctp/sctp.h                            |  3 +-
 include/net/sock.h                                 |  4 +--
 include/net/tcp.h                                  |  4 +--
 include/net/udp.h                                  |  2 +-
 include/sound/hwdep.h                              |  2 +-
 include/sound/info.h                               |  2 +-
 include/uapi/asm-generic/poll.h                    | 36 +++++++++++++---------
 include/uapi/linux/types.h                         |  6 ++++
 ipc/mqueue.c                                       |  4 +--
 kernel/events/core.c                               |  4 +--
 kernel/printk/printk.c                             |  4 +--
 kernel/relay.c                                     |  4 +--
 kernel/time/posix-clock.c                          |  4 +--
 kernel/trace/ring_buffer.c                         |  2 +-
 kernel/trace/trace.c                               |  6 ++--
 mm/memcontrol.c                                    |  2 +-
 mm/swapfile.c                                      |  2 +-
 net/atm/common.c                                   |  4 +--
 net/atm/common.h                                   |  2 +-
 net/batman-adv/debugfs.c                           |  2 +-
 net/batman-adv/icmp_socket.c                       |  2 +-
 net/bluetooth/af_bluetooth.c                       |  7 ++---
 net/caif/caif_socket.c                             |  6 ++--
 net/core/datagram.c                                |  7 ++---
 net/core/sock.c                                    |  2 +-
 net/dccp/dccp.h                                    |  3 +-
 net/dccp/proto.c                                   |  5 ++-
 net/decnet/af_decnet.c                             |  4 +--
 net/ipv4/tcp.c                                     |  4 +--
 net/ipv4/udp.c                                     |  4 +--
 net/irda/af_irda.c                                 |  6 ++--
 net/irda/irnet/irnet_ppp.c                         |  8 ++---
 net/irda/irnet/irnet_ppp.h                         |  2 +-
 net/iucv/af_iucv.c                                 |  8 ++---
 net/netlink/af_netlink.c                           |  6 ++--
 net/nfc/llcp_sock.c                                |  8 ++---
 net/nfc/nci/uart.c                                 |  2 +-
 net/packet/af_packet.c                             |  6 ++--
 net/phonet/socket.c                                |  6 ++--
 net/rds/af_rds.c                                   |  6 ++--
 net/rfkill/core.c                                  |  4 +--
 net/rxrpc/af_rxrpc.c                               |  6 ++--
 net/sctp/socket.c                                  |  4 +--
 net/socket.c                                       |  7 ++---
 net/sunrpc/cache.c                                 | 10 +++---
 net/sunrpc/rpc_pipe.c                              |  4 +--
 net/tipc/socket.c                                  |  6 ++--
 net/unix/af_unix.c                                 | 15 ++++-----
 net/vmw_vsock/af_vsock.c                           |  6 ++--
 security/tomoyo/audit.c                            |  2 +-
 security/tomoyo/common.c                           |  4 +--
 security/tomoyo/common.h                           |  6 ++--
 security/tomoyo/securityfs_if.c                    |  2 +-
 sound/core/compress_offload.c                      |  6 ++--
 sound/core/control.c                               |  4 +--
 sound/core/hwdep.c                                 |  2 +-
 sound/core/info.c                                  |  4 +--
 sound/core/init.c                                  |  2 +-
 sound/core/oss/pcm_oss.c                           |  4 +--
 sound/core/pcm_native.c                            |  8 ++---
 sound/core/rawmidi.c                               |  4 +--
 sound/core/seq/oss/seq_oss.c                       |  4 +--
 sound/core/seq/oss/seq_oss_device.h                |  2 +-
 sound/core/seq/oss/seq_oss_rw.c                    |  4 +--
 sound/core/seq/seq_clientmgr.c                     |  4 +--
 sound/core/timer.c                                 |  4 +--
 sound/firewire/bebob/bebob_hwdep.c                 |  4 +--
 sound/firewire/dice/dice-hwdep.c                   |  4 +--
 sound/firewire/digi00x/digi00x-hwdep.c             |  4 +--
 sound/firewire/fireworks/fireworks_hwdep.c         |  4 +--
 sound/firewire/oxfw/oxfw-hwdep.c                   |  4 +--
 sound/firewire/tascam/tascam-hwdep.c               |  4 +--
 sound/oss/dmabuf.c                                 |  6 ++--
 sound/oss/dmasound/dmasound_core.c                 |  4 +--
 sound/oss/midibuf.c                                |  4 +--
 sound/oss/sequencer.c                              |  4 +--
 sound/oss/sound_calls.h                            |  6 ++--
 sound/oss/soundcard.c                              |  2 +-
 sound/oss/swarm_cs4297a.c                          |  4 +--
 sound/usb/mixer_quirks.c                           |  2 +-
 sound/usb/usx2y/us122l.c                           |  4 +--
 sound/usb/usx2y/usX2Yhwdep.c                       |  4 +--
 virt/kvm/eventfd.c                                 |  4 +--
 379 files changed, 760 insertions(+), 758 deletions(-)
 delete mode 100644 arch/alpha/include/uapi/asm/poll.h
 delete mode 100644 arch/ia64/include/uapi/asm/poll.h
 delete mode 100644 arch/m32r/include/uapi/asm/poll.h
 delete mode 100644 arch/microblaze/include/uapi/asm/poll.h
 delete mode 100644 arch/mn10300/include/uapi/asm/poll.h
 delete mode 100644 arch/powerpc/include/uapi/asm/poll.h
 delete mode 100644 arch/s390/include/uapi/asm/poll.h
 delete mode 100644 arch/score/include/uapi/asm/poll.h
 delete mode 100644 arch/x86/include/uapi/asm/poll.h

^ permalink raw reply	[flat|nested] 12+ messages in thread

* more POLL... fun
  2015-11-30  3:04     ` Al Viro
@ 2015-12-04  6:38       ` Al Viro
  2015-12-04  9:16         ` Arnd Bergmann
  0 siblings, 1 reply; 12+ messages in thread
From: Al Viro @ 2015-12-04  6:38 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: Arnd Bergmann, linuxppc-dev

On cross-builds the __poll_t annotations had caught something interesting:
void spufs_mfc_callback(struct spu *spu)
{
	....
                mask = 0;
                if (free_elements & 0xffff)
                        mask |= POLLOUT;
                if (tagstatus & ctx->tagwait)
                        mask |= POLLIN;

                kill_fasync(&ctx->mfc_fasync, SIGIO, mask);
	....
}

That's arch/powerpc/platforms/cell/spufs/file.c.  WTF is kill_fasync()
getting as the last argument here?  Valid values are
#define POLL_IN         (__SI_POLL|1)   /* data input available */
#define POLL_OUT        (__SI_POLL|2)   /* output buffers available */
#define POLL_MSG        (__SI_POLL|3)   /* input message available */
#define POLL_ERR        (__SI_POLL|4)   /* i/o error */
#define POLL_PRI        (__SI_POLL|5)   /* high priority input available */
#define POLL_HUP        (__SI_POLL|6)   /* device disconnected */

Use of POLLIN, POLLOUT, etc. here is wrong - kill_fasync() will step into
                        BUG_ON((reason & __SI_MASK) != __SI_POLL);
in send_sigio_to_task().  Other two callers of kill_fasync() in that file
are trivially fixed by switching to POLL_IN and POLL_OUT; with this one
I've no idea what had been intended.

What's more, I really wonder if it had _ever_ been tested - these kill_fasync()
calls had been introduced in
commit 8b3d6663c6217e4f50cc3720935a96da9b984117
Author: Arnd Bergmann <arnd@arndb.de>
Date:   Tue Nov 15 15:53:52 2005 -0500

    [PATCH] spufs: cooperative scheduler support
more than 5 years after that BUG_ON() had been added - it goes back to
+                       /* Make sure we are called with one of the POLL_*
+                          reasons, otherwise we could leak kernel stack into
+                          userspace.  */
+                       if ((reason & __SI_MASK) != __SI_POLL)
+                               BUG();
in 2.3.99pre-10-3, on May 25 2000.

What the hell am I missing here?  Has that code been DOA and never used by
anyone in all the decade it had been in mainline?

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: more POLL... fun
  2015-12-04  6:38       ` more POLL... fun Al Viro
@ 2015-12-04  9:16         ` Arnd Bergmann
  2015-12-04 15:21           ` Al Viro
  0 siblings, 1 reply; 12+ messages in thread
From: Arnd Bergmann @ 2015-12-04  9:16 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Al Viro, Linus Torvalds

On Friday 04 December 2015 06:38:25 Al Viro wrote:
> On cross-builds the __poll_t annotations had caught something interesting:
> void spufs_mfc_callback(struct spu *spu)
> {
> 	....
>                 mask = 0;
>                 if (free_elements & 0xffff)
>                         mask |= POLLOUT;
>                 if (tagstatus & ctx->tagwait)
>                         mask |= POLLIN;
> 
>                 kill_fasync(&ctx->mfc_fasync, SIGIO, mask);
> 	....
> }
> 
> That's arch/powerpc/platforms/cell/spufs/file.c.  WTF is kill_fasync()
> getting as the last argument here?  Valid values are
> #define POLL_IN         (__SI_POLL|1)   /* data input available */
> #define POLL_OUT        (__SI_POLL|2)   /* output buffers available */
> #define POLL_MSG        (__SI_POLL|3)   /* input message available */
> #define POLL_ERR        (__SI_POLL|4)   /* i/o error */
> #define POLL_PRI        (__SI_POLL|5)   /* high priority input available */
> #define POLL_HUP        (__SI_POLL|6)   /* device disconnected */
> 
> Use of POLLIN, POLLOUT, etc. here is wrong - kill_fasync() will step into
>                         BUG_ON((reason & __SI_MASK) != __SI_POLL);
> in send_sigio_to_task().  Other two callers of kill_fasync() in that file
> are trivially fixed by switching to POLL_IN and POLL_OUT; with this one
> I've no idea what had been intended.
> 
> What's more, I really wonder if it had _ever_ been tested - these kill_fasync()
> calls had been introduced in
> commit 8b3d6663c6217e4f50cc3720935a96da9b984117
> Author: Arnd Bergmann <arnd@arndb.de>
> Date:   Tue Nov 15 15:53:52 2005 -0500
> 
>     [PATCH] spufs: cooperative scheduler support
> more than 5 years after that BUG_ON() had been added - it goes back to
> +                       /* Make sure we are called with one of the POLL_*
> +                          reasons, otherwise we could leak kernel stack into
> +                          userspace.  */
> +                       if ((reason & __SI_MASK) != __SI_POLL)
> +                               BUG();
> in 2.3.99pre-10-3, on May 25 2000.
> 
> What the hell am I missing here?  Has that code been DOA and never used by
> anyone in all the decade it had been in mainline?

I don't remember why we put in fasync support, but I have checked the libspe
implementation and found that it doesn't use it (not a big surprise there).
It always uses epoll() to get notifications from spufs, and based on your
explanation I assume everything else (there may have been one or two users
that used the low-level interfaces rather than libspe) did too.

	Arnd

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: more POLL... fun
  2015-12-04  9:16         ` Arnd Bergmann
@ 2015-12-04 15:21           ` Al Viro
  2015-12-04 17:13             ` Arnd Bergmann
  0 siblings, 1 reply; 12+ messages in thread
From: Al Viro @ 2015-12-04 15:21 UTC (permalink / raw)
  To: Arnd Bergmann; +Cc: linuxppc-dev, Linus Torvalds

On Fri, Dec 04, 2015 at 10:16:50AM +0100, Arnd Bergmann wrote:

> I don't remember why we put in fasync support, but I have checked the libspe
> implementation and found that it doesn't use it (not a big surprise there).
> It always uses epoll() to get notifications from spufs, and based on your
> explanation I assume everything else (there may have been one or two users
> that used the low-level interfaces rather than libspe) did too.

OK...  So should we just rip ->{mfc,ibox,wbox}_fasync out, along with all
three kill_fasync() and ->fasync() instances in there?  We obviously need to
leave spufs_{mfc,ibox,wbox}_callback() in place for the sake of those
wake_up_all(&ctx->{mfc,ibox,wbox}_wq); in them...

I mean, fasync in there obviously never been used at all - it never delivered
a single SIGIO, and the first user to try would get the BUG_ON() in fcntl.c
instead.  Since nobody complained in more than 10 years...

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: more POLL... fun
  2015-12-04 15:21           ` Al Viro
@ 2015-12-04 17:13             ` Arnd Bergmann
  2015-12-04 17:30               ` Al Viro
  0 siblings, 1 reply; 12+ messages in thread
From: Arnd Bergmann @ 2015-12-04 17:13 UTC (permalink / raw)
  To: Al Viro; +Cc: linuxppc-dev, Linus Torvalds

On Friday 04 December 2015 15:21:33 Al Viro wrote:
> On Fri, Dec 04, 2015 at 10:16:50AM +0100, Arnd Bergmann wrote:
> 
> > I don't remember why we put in fasync support, but I have checked the libspe
> > implementation and found that it doesn't use it (not a big surprise there).
> > It always uses epoll() to get notifications from spufs, and based on your
> > explanation I assume everything else (there may have been one or two users
> > that used the low-level interfaces rather than libspe) did too.
> 
> OK...  So should we just rip ->{mfc,ibox,wbox}_fasync out, along with all
> three kill_fasync() and ->fasync() instances in there?  We obviously need to
> leave spufs_{mfc,ibox,wbox}_callback() in place for the sake of those
> wake_up_all(&ctx->{mfc,ibox,wbox}_wq); in them...
> 
> I mean, fasync in there obviously never been used at all - it never delivered
> a single SIGIO, and the first user to try would get the BUG_ON() in fcntl.c
> instead.  Since nobody complained in more than 10 years...

Yes, I think that would be best. Do you want me to send that patch, or do
you prefer to do it yourself? In theory that patch should also go into stable
kernels, but I suspect nobody who still owns a machine that is able to run
this code will ever upgrade to a stable release, so we probably don't need
that.

	Arnd

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: more POLL... fun
  2015-12-04 17:13             ` Arnd Bergmann
@ 2015-12-04 17:30               ` Al Viro
  2015-12-04 17:34                 ` Arnd Bergmann
                                   ` (2 more replies)
  0 siblings, 3 replies; 12+ messages in thread
From: Al Viro @ 2015-12-04 17:30 UTC (permalink / raw)
  To: Arnd Bergmann; +Cc: linuxppc-dev, Linus Torvalds

On Fri, Dec 04, 2015 at 06:13:54PM +0100, Arnd Bergmann wrote:

> Yes, I think that would be best. Do you want me to send that patch, or do
> you prefer to do it yourself? In theory that patch should also go into stable
> kernels, but I suspect nobody who still owns a machine that is able to run
> this code will ever upgrade to a stable release, so we probably don't need
> that.

Probably better if it goes through ppc tree - the only relationship it has
to VFS is broken calls of kill_fasync() in now-removed code...  Something
like this, perhaps?

[spufs] get rid of broken fasync stuff
 
In all the years it's been in the tree it had never been used by
anything - it would instantly trigger BUG_ON() in fs/fcntl.c due
to bogus band argument passed to kill_fasync().  Since nobody
had ever used it in ten years, let's just rip it out and be
done with that.

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---

diff --git a/arch/powerpc/platforms/cell/spufs/file.c b/arch/powerpc/platforms/cell/spufs/file.c
index 5038fd5..88b7613 100644
--- a/arch/powerpc/platforms/cell/spufs/file.c
+++ b/arch/powerpc/platforms/cell/spufs/file.c
@@ -686,23 +686,13 @@ size_t spu_ibox_read(struct spu_context *ctx, u32 *data)
 	return ctx->ops->ibox_read(ctx, data);
 }
 
-static int spufs_ibox_fasync(int fd, struct file *file, int on)
-{
-	struct spu_context *ctx = file->private_data;
-
-	return fasync_helper(fd, file, on, &ctx->ibox_fasync);
-}
-
 /* interrupt-level ibox callback function. */
 void spufs_ibox_callback(struct spu *spu)
 {
 	struct spu_context *ctx = spu->ctx;
 
-	if (!ctx)
-		return;
-
-	wake_up_all(&ctx->ibox_wq);
-	kill_fasync(&ctx->ibox_fasync, SIGIO, POLLIN);
+	if (ctx)
+		wake_up_all(&ctx->ibox_wq);
 }
 
 /*
@@ -797,7 +787,6 @@ static const struct file_operations spufs_ibox_fops = {
 	.open	= spufs_pipe_open,
 	.read	= spufs_ibox_read,
 	.poll	= spufs_ibox_poll,
-	.fasync	= spufs_ibox_fasync,
 	.llseek = no_llseek,
 };
 
@@ -835,26 +824,13 @@ size_t spu_wbox_write(struct spu_context *ctx, u32 data)
 	return ctx->ops->wbox_write(ctx, data);
 }
 
-static int spufs_wbox_fasync(int fd, struct file *file, int on)
-{
-	struct spu_context *ctx = file->private_data;
-	int ret;
-
-	ret = fasync_helper(fd, file, on, &ctx->wbox_fasync);
-
-	return ret;
-}
-
 /* interrupt-level wbox callback function. */
 void spufs_wbox_callback(struct spu *spu)
 {
 	struct spu_context *ctx = spu->ctx;
 
-	if (!ctx)
-		return;
-
-	wake_up_all(&ctx->wbox_wq);
-	kill_fasync(&ctx->wbox_fasync, SIGIO, POLLOUT);
+	if (ctx)
+		wake_up_all(&ctx->wbox_wq);
 }
 
 /*
@@ -947,7 +923,6 @@ static const struct file_operations spufs_wbox_fops = {
 	.open	= spufs_pipe_open,
 	.write	= spufs_wbox_write,
 	.poll	= spufs_wbox_poll,
-	.fasync	= spufs_wbox_fasync,
 	.llseek = no_llseek,
 };
 
@@ -1523,28 +1498,8 @@ void spufs_mfc_callback(struct spu *spu)
 {
 	struct spu_context *ctx = spu->ctx;
 
-	if (!ctx)
-		return;
-
-	wake_up_all(&ctx->mfc_wq);
-
-	pr_debug("%s %s\n", __func__, spu->name);
-	if (ctx->mfc_fasync) {
-		u32 free_elements, tagstatus;
-		unsigned int mask;
-
-		/* no need for spu_acquire in interrupt context */
-		free_elements = ctx->ops->get_mfc_free_elements(ctx);
-		tagstatus = ctx->ops->read_mfc_tagstatus(ctx);
-
-		mask = 0;
-		if (free_elements & 0xffff)
-			mask |= POLLOUT;
-		if (tagstatus & ctx->tagwait)
-			mask |= POLLIN;
-
-		kill_fasync(&ctx->mfc_fasync, SIGIO, mask);
-	}
+	if (ctx)
+		wake_up_all(&ctx->mfc_wq);
 }
 
 static int spufs_read_mfc_tagstatus(struct spu_context *ctx, u32 *status)
@@ -1806,13 +1761,6 @@ static int spufs_mfc_fsync(struct file *file, loff_t start, loff_t end, int data
 	return err;
 }
 
-static int spufs_mfc_fasync(int fd, struct file *file, int on)
-{
-	struct spu_context *ctx = file->private_data;
-
-	return fasync_helper(fd, file, on, &ctx->mfc_fasync);
-}
-
 static const struct file_operations spufs_mfc_fops = {
 	.open	 = spufs_mfc_open,
 	.release = spufs_mfc_release,
@@ -1821,7 +1769,6 @@ static const struct file_operations spufs_mfc_fops = {
 	.poll	 = spufs_mfc_poll,
 	.flush	 = spufs_mfc_flush,
 	.fsync	 = spufs_mfc_fsync,
-	.fasync	 = spufs_mfc_fasync,
 	.mmap	 = spufs_mfc_mmap,
 	.llseek  = no_llseek,
 };
diff --git a/arch/powerpc/platforms/cell/spufs/spufs.h b/arch/powerpc/platforms/cell/spufs/spufs.h
index bcfd6f0..aac7339 100644
--- a/arch/powerpc/platforms/cell/spufs/spufs.h
+++ b/arch/powerpc/platforms/cell/spufs/spufs.h
@@ -102,9 +102,6 @@ struct spu_context {
 	wait_queue_head_t stop_wq;
 	wait_queue_head_t mfc_wq;
 	wait_queue_head_t run_wq;
-	struct fasync_struct *ibox_fasync;
-	struct fasync_struct *wbox_fasync;
-	struct fasync_struct *mfc_fasync;
 	u32 tagwait;
 	struct spu_context_ops *ops;
 	struct work_struct reap_work;

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: more POLL... fun
  2015-12-04 17:30               ` Al Viro
@ 2015-12-04 17:34                 ` Arnd Bergmann
  2015-12-06 23:58                 ` Michael Ellerman
  2017-02-16  5:59                 ` Michael Ellerman
  2 siblings, 0 replies; 12+ messages in thread
From: Arnd Bergmann @ 2015-12-04 17:34 UTC (permalink / raw)
  To: Al Viro; +Cc: linuxppc-dev, Linus Torvalds, mpe

On Friday 04 December 2015 17:30:31 Al Viro wrote:
> On Fri, Dec 04, 2015 at 06:13:54PM +0100, Arnd Bergmann wrote:
> 
> > Yes, I think that would be best. Do you want me to send that patch, or do
> > you prefer to do it yourself? In theory that patch should also go into stable
> > kernels, but I suspect nobody who still owns a machine that is able to run
> > this code will ever upgrade to a stable release, so we probably don't need
> > that.
> 
> Probably better if it goes through ppc tree - the only relationship it has
> to VFS is broken calls of kill_fasync() in now-removed code...

Right.

>  Something like this, perhaps?
>
> [spufs] get rid of broken fasync stuff
>  
> In all the years it's been in the tree it had never been used by
> anything - it would instantly trigger BUG_ON() in fs/fcntl.c due
> to bogus band argument passed to kill_fasync().  Since nobody
> had ever used it in ten years, let's just rip it out and be
> done with that.
> 
> Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
> ---

Looks all good, thanks!

Acked-by: Arnd Bergmann <arnd@arndb.de>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: more POLL... fun
  2015-12-04 17:30               ` Al Viro
  2015-12-04 17:34                 ` Arnd Bergmann
@ 2015-12-06 23:58                 ` Michael Ellerman
  2017-02-16  5:59                 ` Michael Ellerman
  2 siblings, 0 replies; 12+ messages in thread
From: Michael Ellerman @ 2015-12-06 23:58 UTC (permalink / raw)
  To: Al Viro, Arnd Bergmann; +Cc: Linus Torvalds, linuxppc-dev

On Fri, 2015-12-04 at 17:30 +0000, Al Viro wrote:
> On Fri, Dec 04, 2015 at 06:13:54PM +0100, Arnd Bergmann wrote:
> > Yes, I think that would be best. Do you want me to send that patch, or do
> > you prefer to do it yourself? In theory that patch should also go into stable
> > kernels, but I suspect nobody who still owns a machine that is able to run
> > this code will ever upgrade to a stable release, so we probably don't need
> > that.
> 
> Probably better if it goes through ppc tree - the only relationship it has
> to VFS is broken calls of kill_fasync() in now-removed code...  Something
> like this, perhaps?

Yeah that looks fine to me. I'll take it into powerpc#next for 4.5.

cheers

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: more POLL... fun
  2015-12-04 17:30               ` Al Viro
  2015-12-04 17:34                 ` Arnd Bergmann
  2015-12-06 23:58                 ` Michael Ellerman
@ 2017-02-16  5:59                 ` Michael Ellerman
  2 siblings, 0 replies; 12+ messages in thread
From: Michael Ellerman @ 2017-02-16  5:59 UTC (permalink / raw)
  To: Al Viro, Arnd Bergmann; +Cc: Linus Torvalds, linuxppc-dev

On Fri, 2015-12-04 at 17:30:31 UTC, Al Viro wrote:
> On Fri, Dec 04, 2015 at 06:13:54PM +0100, Arnd Bergmann wrote:
> 
> > Yes, I think that would be best. Do you want me to send that patch, or do
> > you prefer to do it yourself? In theory that patch should also go into stable
> > kernels, but I suspect nobody who still owns a machine that is able to run
> > this code will ever upgrade to a stable release, so we probably don't need
> > that.
> 
> Probably better if it goes through ppc tree - the only relationship it has
> to VFS is broken calls of kill_fasync() in now-removed code...  Something
> like this, perhaps?
> 
> [spufs] get rid of broken fasync stuff
>  
> In all the years it's been in the tree it had never been used by
> anything - it would instantly trigger BUG_ON() in fs/fcntl.c due
> to bogus band argument passed to kill_fasync().  Since nobody
> had ever used it in ten years, let's just rip it out and be
> done with that.
> 
> Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
> Acked-by: Arnd Bergmann <arnd@arndb.de>

Applied to powerpc next, thanks.

https://git.kernel.org/powerpc/c/7d7be3aa08fa338e84eb235805ee18

cheers

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2017-02-16  5:59 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-11-27  5:00 ->poll() instances shouldn't be indefinitely blocking Al Viro
2015-11-27 15:18 ` Mauro Carvalho Chehab
2015-11-27 17:49   ` Linus Torvalds
2015-11-30  3:04     ` Al Viro
2015-12-04  6:38       ` more POLL... fun Al Viro
2015-12-04  9:16         ` Arnd Bergmann
2015-12-04 15:21           ` Al Viro
2015-12-04 17:13             ` Arnd Bergmann
2015-12-04 17:30               ` Al Viro
2015-12-04 17:34                 ` Arnd Bergmann
2015-12-06 23:58                 ` Michael Ellerman
2017-02-16  5:59                 ` Michael Ellerman

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.