linux.git/fs/sysfs/file.c, branch v3.13

sysfs: give different locking key to regular and bin files

2013-12-08T05:22:00+00:00

027a485d12e0 ("sysfs: use a separate locking class for open files
depending on mmap") assigned different lockdep key to
sysfs_open_file->mutex depending on whether the file implements mmap
or not in an attempt to avoid spurious lockdep warning caused by
merging of regular and bin file paths.

While this restored some of the original behavior of using different
locks (at least lockdep is concerned) for the different clases of
files.  The restoration wasn't full because now the lockdep key
assignment depends on whether the file has mmap or not instead of
whether it's a regular file or not.

This means that bin files which don't implement mmap will get assigned
the same lockdep class as regular files.  This is problematic because
file_operations for bin files still implements the mmap file operation
and checking whether the sysfs file actually implements mmap happens
in the file operation after grabbing @sysfs_open_file->mutex.  We
still end up adding locking dependency from mmap locking to
sysfs_open_file->mutex to the regular file mutex which triggers
spurious circular locking warning.

Fix it by restoring the original behavior fully by differentiating
lockdep key by whether the file is regular or bin, instead of the
existence of mmap.

Signed-off-by: Tejun Heo 
Reported-by: Dave Jones 
Link: http://lkml.kernel.org/g/20131203184324.GA11320@redhat.com
Signed-off-by: Greg Kroah-Hartman

sysfs: use a separate locking class for open files depending on mmap

2013-11-23T18:52:13+00:00

The following two commits implemented mmap support in the regular file
path and merged bin file support into the regular path.

 73d9714627ad ("sysfs: copy bin mmap support from fs/sysfs/bin.c to fs/sysfs/file.c")
 3124eb1679b2 ("sysfs: merge regular and bin file handling")

After the merge, the following commands trigger a spurious lockdep
warning.  "test-mmap-read" simply mmaps the file and dumps the
content.

  $ cat /sys/block/sda/trace/act_mask
  $ test-mmap-read /sys/devices/pci0000\:00/0000\:00\:03.0/resource0 4096

  ======================================================
  [ INFO: possible circular locking dependency detected ]
  3.12.0-work+ #378 Not tainted
  -------------------------------------------------------
  test-mmap-read/567 is trying to acquire lock:
   (&of->mutex){+.+.+.}, at: [] sysfs_bin_mmap+0x4f/0x120

  but task is already holding lock:
   (&mm->mmap_sem){++++++}, at: [] vm_mmap_pgoff+0x49/0xa0

  which lock already depends on the new lock.

  the existing dependency chain (in reverse order) is:

  -> #3 (&mm->mmap_sem){++++++}:
  ...
  -> #2 (sr_mutex){+.+.+.}:
  ...
  -> #1 (&bdev->bd_mutex){+.+.+.}:
  ...
  -> #0 (&of->mutex){+.+.+.}:
  ...

  other info that might help us debug this:

  Chain exists of:
   &of->mutex --> sr_mutex --> &mm->mmap_sem

   Possible unsafe locking scenario:

	 CPU0                    CPU1
	 ----                    ----
    lock(&mm->mmap_sem);
				 lock(sr_mutex);
				 lock(&mm->mmap_sem);
    lock(&of->mutex);

   *** DEADLOCK ***

  1 lock held by test-mmap-read/567:
   #0:  (&mm->mmap_sem){++++++}, at: [] vm_mmap_pgoff+0x49/0xa0

  stack backtrace:
  CPU: 3 PID: 567 Comm: test-mmap-read Not tainted 3.12.0-work+ #378
  Hardware name: Bochs Bochs, BIOS Bochs 01/01/2011
   ffffffff81ed41a0 ffff880009441bc8 ffffffff81611ad2 ffffffff81eccb80
   ffff880009441c08 ffffffff8160f215 ffff880009441c60 ffff880009c75208
   0000000000000000 ffff880009c751e0 ffff880009c75208 ffff880009c74ac0
  Call Trace:
   [] dump_stack+0x4e/0x7a
   [] print_circular_bug+0x2b0/0x2bf
   [] __lock_acquire+0x1a3a/0x1e60
   [] lock_acquire+0x9a/0x1d0
   [] mutex_lock_nested+0x67/0x3f0
   [] sysfs_bin_mmap+0x4f/0x120
   [] mmap_region+0x3b3/0x5b0
   [] do_mmap_pgoff+0x34e/0x3d0
   [] vm_mmap_pgoff+0x6a/0xa0
   [] SyS_mmap_pgoff+0xbe/0x250
   [] SyS_mmap+0x22/0x30
   [] system_call_fastpath+0x16/0x1b

This happens because one file nests sr_mutex, which nests mm->mmap_sem
under it, under of->mutex while mmap implementation naturally nests
of->mutex under mm->mmap_sem.  The warning is false positive as
of->mutex is per open-file and the two paths belong to two different
files.  This warning didn't trigger before regular and bin file
supports were merged because only bin file supported mmap and the
other side of locking happened only on regular files which used
equivalent but separate locking.

It'd be best if we give separate locking classes per file but we can't
easily do that.  Let's differentiate on ->mmap() for now.  Later we'll
add explicit file operations struct and can add per-ops lockdep key
there.

Signed-off-by: Tejun Heo 
Reported-by: Dave Jones 
Signed-off-by: Greg Kroah-Hartman

sysfs: use generic_file_llseek() for sysfs_file_operations

2013-11-01T19:13:37+00:00

13c589d5b0ac6 ("sysfs: use seq_file when reading regular files")
converted regular sysfs files to use seq_file.  The commit substituted
generic_file_llseek() with seq_lseek() for llseek implementation.

Before the change, all regular sysfs files were allowed to seek to any
position in [0, PAGE_SIZE] as the file size is always PAGE_SIZE and
generic_file_llseek() allows any seeking inside the range under file
size; however, seq_lseek()'s behavior is different.  It traverses the
output by repeatedly invoking ->show() until it reaches the target
offset or traversal indicates EOF.  As seq_files are fully dynamic and
may not end at all, it doesn't support seeking from the end
(SEEK_END).

Apparently, there are userland tools which uses SEEK_END to discover
the buffer size to use and the switch to seq_lseek() disturbs them as
SEEK_END fails with -EINVAL.

The only benefits of using seq_lseek() instead of
generic_file_llseek() are

* Early failure.  If traversing to certain file position should fail,
  seq_lseek() will report such failures on lseek(2) instead of the
  following read/write operations.

* EOF detection.  While SEEK_END is not supported, SEEK_SET/CUR +
  large offset can be used to detect eof - eof at the time of the seek
  anyway as the file size may change dynamically.

Both aren't necessary for sysfs or prospect kernfs users.  Revert to
genefic_file_llseek() and preserve the original behavior.

Signed-off-by: Tejun Heo 
Reported-by: Heiko Carstens 
Link: https://lkml.kernel.org/r/20131031114358.GA5551@osiris
Tested-by: Heiko Carstens 
Signed-off-by: Greg Kroah-Hartman

sysfs: return correct error code on unimplemented mmap()

2013-10-30T17:21:39+00:00

Both POSIX.1-2008 and Linux Programmer's Manual have a dedicated return
error code for a case, when a file doesn't support mmap(), it's ENODEV.

This change replaces overloaded EINVAL with ENODEV in a situation
described above for sysfs binary files.

Signed-off-by: Vladimir Zapolskiy 
Signed-off-by: Greg Kroah-Hartman

sysfs: merge sysfs_elem_bin_attr into sysfs_elem_attr

2013-10-29T22:12:06+00:00

3124eb1679 ("sysfs: merge regular and bin file handling") folded bin
file handling into regular file handling.  Among other things, bin
file now shares the same open path including sysfs_open_dirent
association using sysfs_dirent->s_attr.open.  This is buggy because
->s_bin_attr lives in the same union and doesn't have the field.  This
bug doesn't trigger because sysfs_elem_bin_attr doesn't have an active
field at the conflicting position.  It does have a field "buffers" but
it isn't used anymore.

This patch collapses sysfs_elem_bin_attr into sysfs_elem_attr so that
the bin_attr is accessed through ->s_attr.bin_attr which lives with
->s_attr.attr in an anonymous union.  The code paths already assume
bin_attr contains attr as the first element, so this doesn't add any
more assumptions while making it explicit that the two types are
handled together.

Signed-off-by: Tejun Heo 
Signed-off-by: Greg Kroah-Hartman

sysfs: fix sysfs_write_file for bin file

2013-10-25T04:46:27+00:00

Before patch(sysfs: prepare path write for unified regular / bin
file handling), when size of bin file is zero, writting still can
continue, but this patch changes the behaviour.

The worse thing is that firmware loader is broken by this patch,
and user space application can't write to firmware bin file any more
because both firmware loader and drivers can't know at advance how
large the firmware file is and have to set its initialized size as
zero.

This patch fixes the problem and keeps behaviour of writting to bin
as before.

Reported-by: Lothar Waßmann 
Tested-by: Lothar Waßmann 
Acked-by: Tejun Heo 
Signed-off-by: Ming Lei 
Signed-off-by: Greg Kroah-Hartman

sysfs/bin: Fix size handling overflow for bin_attribute

2013-10-14T17:07:19+00:00

While looking at the code, I noticed that bin_attribute read() and write()
ops copy the inode size into an int for futher comparisons.

Some bin_attributes can be fairly large. For example, pci creates some for
BARs set to the BAR size and giant BARs are around the corner, so this is
going to break something somewhere eventually.

Let's use the right type.

[adjust for seqfile conversions, only needed for bin_read() - gkh]

Signed-off-by: Benjamin Herrenschmidt 
Cc: Tejun Heo 
Signed-off-by: Greg Kroah-Hartman

sysfs: make sysfs_file_ops() follow ignore_lockdep flag

2013-10-14T15:40:39+00:00

375b611e60 ("sysfs: remove sysfs_buffer->ops") introduced
sysfs_file_ops() which determines the associated file operation of a
given sysfs_dirent.  As file ops access should be protected by an
active reference, the new function includes a lockdep assertion on the
sysfs_dirent; unfortunately, I forgot to take attr->ignore_lockdep
flag into account and the lockdep assertion trips spuriously for files
which opt out from active reference lockdep checking.

# cat /sys/devices/pci0000:00/0000:00:01.2/usb1/authorized

 ------------[ cut here ]------------
 WARNING: CPU: 1 PID: 540 at /work/os/work/fs/sysfs/file.c:79 sysfs_file_ops+0x4e/0x60()
 Modules linked in:
 CPU: 1 PID: 540 Comm: cat Not tainted 3.11.0-work+ #3
 Hardware name: Bochs Bochs, BIOS Bochs 01/01/2011
  0000000000000009 ffff880016205c08 ffffffff81ca0131 0000000000000000
  ffff880016205c40 ffffffff81096d0d ffff8800166cb898 ffff8800166f6f60
  ffffffff8125a220 ffff880011ab1ec0 ffff88000aff0c78 ffff880016205c50
 Call Trace:
  [] dump_stack+0x4e/0x82
  [] warn_slowpath_common+0x7d/0xa0
  [] warn_slowpath_null+0x1a/0x20
  [] sysfs_file_ops+0x4e/0x60
  [] sysfs_open_file+0x54/0x300
  [] do_dentry_open.isra.17+0x182/0x280
  [] finish_open+0x30/0x40
  [] do_last+0x503/0xd90
  [] path_openat+0xbb/0x6d0
  [] do_filp_open+0x3a/0x90
  [] do_sys_open+0x129/0x220
  [] SyS_open+0x1e/0x20
  [] system_call_fastpath+0x16/0x1b
 ---[ end trace aa48096b111dafdb ]---

Rename fs/sysfs/dir.c::ignore_lockdep() to sysfs_ignore_lockdep() and
move it to fs/sysfs/sysfs.h and make sysfs_file_ops() skip lockdep
assertion if sysfs_ignore_lockdep() is true.

Signed-off-by: Tejun Heo 
Reported-by: Yinghai Lu 
Signed-off-by: Greg Kroah-Hartman

sysfs: merge regular and bin file handling

2013-10-06T00:27:40+00:00

With the previous changes, sysfs regular file code is ready to handle
bin files too.  This patch makes bin files share the regular file
path.

* sysfs_create/remove_bin_file() are moved to fs/sysfs/file.c.

* sysfs_init_inode() is updated to use the new sysfs_bin_operations
  instead of bin_fops for bin files.

* fs/sysfs/bin.c and the related pieces are removed.

This patch shouldn't introduce any behavior difference to bin file
accesses.

Overall, this unification reduces the amount of duplicate logic, makes
behaviors more consistent and paves the road for building simpler and
more versatile interface which will allow other subsystems to make use
of sysfs for their pseudo filesystems.

v2: Stale fs/sysfs/bin.c reference dropped from
    Documentation/DocBook/filesystems.tmpl.  Reported by kbuild test
    robot.

Signed-off-by: Tejun Heo 
Cc: Kay Sievers 
Cc: kbuild test robot 
Signed-off-by: Greg Kroah-Hartman

sysfs: prepare open path for unified regular / bin file handling

2013-10-06T00:27:40+00:00

sysfs bin file handling will be merged into the regular file support.
This patch prepares the open path.

This patch updates sysfs_open_file() such that it can handle both
regular and bin files.

This is a preparation and the new bin file path isn't used yet.

Signed-off-by: Tejun Heo 
Signed-off-by: Greg Kroah-Hartman