05 Sep, 2018

2 commits

  • [ Upstream commit c2412ac45a8f8f1cd582723c1a139608694d410d ]

    If we meet a conflicting object that is marked FSCACHE_OBJECT_IS_LIVE in
    the active object tree, we have been emitting a BUG after logging
    information about it and the new object.

    Instead, we should wait for the CACHEFILES_OBJECT_ACTIVE flag to be cleared
    on the old object (or return an error). The ACTIVE flag should be cleared
    after it has been removed from the active object tree. A timeout of 60s is
    used in the wait, so we shouldn't be able to get stuck there.

    Fixes: 9ae326a69004 ("CacheFiles: A cache that backs onto a mounted filesystem")
    Signed-off-by: Kiran Kumar Modukuri
    Signed-off-by: David Howells
    Signed-off-by: Sasha Levin
    Signed-off-by: Greg Kroah-Hartman

    Kiran Kumar Modukuri
     
  • [ Upstream commit 934140ab028713a61de8bca58c05332416d037d1 ]

    cachefiles_read_waiter() has the right to access a 'monitor' object by
    virtue of being called under the waitqueue lock for one of the pages in its
    purview. However, it has no ref on that monitor object or on the
    associated operation.

    What it is allowed to do is to move the monitor object to the operation's
    to_do list, but once it drops the work_lock, it's actually no longer
    permitted to access that object. However, it is trying to enqueue the
    retrieval operation for processing - but it can only do this via a pointer
    in the monitor object, something it shouldn't be doing.

    If it doesn't enqueue the operation, the operation may not get processed.
    If the order is flipped so that the enqueue is first, then it's possible
    for the work processor to look at the to_do list before the monitor is
    enqueued upon it.

    Fix this by getting a ref on the operation so that we can trust that it
    will still be there once we've added the monitor to the to_do list and
    dropped the work_lock. The op can then be enqueued after the lock is
    dropped.

    The bug can manifest in one of a couple of ways. The first manifestation
    looks like:

    FS-Cache:
    FS-Cache: Assertion failed
    FS-Cache: 6 == 5 is false
    ------------[ cut here ]------------
    kernel BUG at fs/fscache/operation.c:494!
    RIP: 0010:fscache_put_operation+0x1e3/0x1f0
    ...
    fscache_op_work_func+0x26/0x50
    process_one_work+0x131/0x290
    worker_thread+0x45/0x360
    kthread+0xf8/0x130
    ? create_worker+0x190/0x190
    ? kthread_cancel_work_sync+0x10/0x10
    ret_from_fork+0x1f/0x30

    This is due to the operation being in the DEAD state (6) rather than
    INITIALISED, COMPLETE or CANCELLED (5) because it's already passed through
    fscache_put_operation().

    The bug can also manifest like the following:

    kernel BUG at fs/fscache/operation.c:69!
    ...
    [exception RIP: fscache_enqueue_operation+246]
    ...
    #7 [ffff883fff083c10] fscache_enqueue_operation at ffffffffa0b793c6
    #8 [ffff883fff083c28] cachefiles_read_waiter at ffffffffa0b15a48
    #9 [ffff883fff083c48] __wake_up_common at ffffffff810af028

    I'm not entirely certain as to which is line 69 in Lei's kernel, so I'm not
    entirely clear which assertion failed.

    Fixes: 9ae326a69004 ("CacheFiles: A cache that backs onto a mounted filesystem")
    Reported-by: Lei Xue
    Reported-by: Vegard Nossum
    Reported-by: Anthony DeRobertis
    Reported-by: NeilBrown
    Reported-by: Daniel Axtens
    Reported-by: Kiran Kumar Modukuri
    Signed-off-by: David Howells
    Reviewed-by: Daniel Axtens
    Signed-off-by: Sasha Levin
    Signed-off-by: Greg Kroah-Hartman

    Kiran Kumar Modukuri
     

02 Nov, 2017

1 commit

  • Many source files in the tree are missing licensing information, which
    makes it harder for compliance tools to determine the correct license.

    By default all files without license information are under the default
    license of the kernel, which is GPL version 2.

    Update the files which contain no license information with the 'GPL-2.0'
    SPDX license identifier. The SPDX identifier is a legally binding
    shorthand, which can be used instead of the full boiler plate text.

    This patch is based on work done by Thomas Gleixner and Kate Stewart and
    Philippe Ombredanne.

    How this work was done:

    Patches were generated and checked against linux-4.14-rc6 for a subset of
    the use cases:
    - file had no licensing information it it.
    - file was a */uapi/* one with no licensing information in it,
    - file was a */uapi/* one with existing licensing information,

    Further patches will be generated in subsequent months to fix up cases
    where non-standard license headers were used, and references to license
    had to be inferred by heuristics based on keywords.

    The analysis to determine which SPDX License Identifier to be applied to
    a file was done in a spreadsheet of side by side results from of the
    output of two independent scanners (ScanCode & Windriver) producing SPDX
    tag:value files created by Philippe Ombredanne. Philippe prepared the
    base worksheet, and did an initial spot review of a few 1000 files.

    The 4.13 kernel was the starting point of the analysis with 60,537 files
    assessed. Kate Stewart did a file by file comparison of the scanner
    results in the spreadsheet to determine which SPDX license identifier(s)
    to be applied to the file. She confirmed any determination that was not
    immediately clear with lawyers working with the Linux Foundation.

    Criteria used to select files for SPDX license identifier tagging was:
    - Files considered eligible had to be source code files.
    - Make and config files were included as candidates if they contained >5
    lines of source
    - File already had some variant of a license header in it (even if
    Reviewed-by: Philippe Ombredanne
    Reviewed-by: Thomas Gleixner
    Signed-off-by: Greg Kroah-Hartman

    Greg Kroah-Hartman
     

17 Jul, 2017

1 commit

  • Firstly by applying the following with coccinelle's spatch:

    @@ expression SB; @@
    -SB->s_flags & MS_RDONLY
    +sb_rdonly(SB)

    to effect the conversion to sb_rdonly(sb), then by applying:

    @@ expression A, SB; @@
    (
    -(!sb_rdonly(SB)) && A
    +!sb_rdonly(SB) && A
    |
    -A != (sb_rdonly(SB))
    +A != sb_rdonly(SB)
    |
    -A == (sb_rdonly(SB))
    +A == sb_rdonly(SB)
    |
    -!(sb_rdonly(SB))
    +!sb_rdonly(SB)
    |
    -A && (sb_rdonly(SB))
    +A && sb_rdonly(SB)
    |
    -A || (sb_rdonly(SB))
    +A || sb_rdonly(SB)
    |
    -(sb_rdonly(SB)) != A
    +sb_rdonly(SB) != A
    |
    -(sb_rdonly(SB)) == A
    +sb_rdonly(SB) == A
    |
    -(sb_rdonly(SB)) && A
    +sb_rdonly(SB) && A
    |
    -(sb_rdonly(SB)) || A
    +sb_rdonly(SB) || A
    )

    @@ expression A, B, SB; @@
    (
    -(sb_rdonly(SB)) ? 1 : 0
    +sb_rdonly(SB)
    |
    -(sb_rdonly(SB)) ? A : B
    +sb_rdonly(SB) ? A : B
    )

    to remove left over excess bracketage and finally by applying:

    @@ expression A, SB; @@
    (
    -(A & MS_RDONLY) != sb_rdonly(SB)
    +(bool)(A & MS_RDONLY) != sb_rdonly(SB)
    |
    -(A & MS_RDONLY) == sb_rdonly(SB)
    +(bool)(A & MS_RDONLY) == sb_rdonly(SB)
    )

    to make comparisons against the result of sb_rdonly() (which is a bool)
    work correctly.

    Signed-off-by: David Howells

    David Howells
     

20 Jun, 2017

3 commits

  • So I've noticed a number of instances where it was not obvious from the
    code whether ->task_list was for a wait-queue head or a wait-queue entry.

    Furthermore, there's a number of wait-queue users where the lists are
    not for 'tasks' but other entities (poll tables, etc.), in which case
    the 'task_list' name is actively confusing.

    To clear this all up, name the wait-queue head and entry list structure
    fields unambiguously:

    struct wait_queue_head::task_list => ::head
    struct wait_queue_entry::task_list => ::entry

    For example, this code:

    rqw->wait.task_list.next != &wait->task_list

    ... is was pretty unclear (to me) what it's doing, while now it's written this way:

    rqw->wait.head.next != &wait->entry

    ... which makes it pretty clear that we are iterating a list until we see the head.

    Other examples are:

    list_for_each_entry_safe(pos, next, &x->task_list, task_list) {
    list_for_each_entry(wq, &fence->wait.task_list, task_list) {

    ... where it's unclear (to me) what we are iterating, and during review it's
    hard to tell whether it's trying to walk a wait-queue entry (which would be
    a bug), while now it's written as:

    list_for_each_entry_safe(pos, next, &x->head, entry) {
    list_for_each_entry(wq, &fence->wait.head, entry) {

    Cc: Linus Torvalds
    Cc: Peter Zijlstra
    Cc: Thomas Gleixner
    Cc: linux-kernel@vger.kernel.org
    Signed-off-by: Ingo Molnar

    Ingo Molnar
     
  • The wait_bit*() types and APIs are mixed into wait.h, but they
    are a pretty orthogonal extension of wait-queues.

    Furthermore, only about 50 kernel files use these APIs, while
    over 1000 use the regular wait-queue functionality.

    So clean up the main wait.h by moving the wait-bit functionality
    out of it, into a separate .h and .c file:

    include/linux/wait_bit.h for types and APIs
    kernel/sched/wait_bit.c for the implementation

    Update all header dependencies.

    This reduces the size of wait.h rather significantly, by about 30%.

    Cc: Linus Torvalds
    Cc: Peter Zijlstra
    Cc: Thomas Gleixner
    Cc: linux-kernel@vger.kernel.org
    Signed-off-by: Ingo Molnar

    Ingo Molnar
     
  • Rename:

    wait_queue_t => wait_queue_entry_t

    'wait_queue_t' was always a slight misnomer: its name implies that it's a "queue",
    but in reality it's a queue *entry*. The 'real' queue is the wait queue head,
    which had to carry the name.

    Start sorting this out by renaming it to 'wait_queue_entry_t'.

    This also allows the real structure name 'struct __wait_queue' to
    lose its double underscore and become 'struct wait_queue_entry',
    which is the more canonical nomenclature for such data types.

    Cc: Linus Torvalds
    Cc: Peter Zijlstra
    Cc: Thomas Gleixner
    Cc: linux-kernel@vger.kernel.org
    Signed-off-by: Ingo Molnar

    Ingo Molnar
     

02 Mar, 2017

1 commit


11 Oct, 2016

2 commits

  • Pull more vfs updates from Al Viro:
    ">rename2() work from Miklos + current_time() from Deepa"

    * 'for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/viro/vfs:
    fs: Replace current_fs_time() with current_time()
    fs: Replace CURRENT_TIME_SEC with current_time() for inode timestamps
    fs: Replace CURRENT_TIME with current_time() for inode timestamps
    fs: proc: Delete inode time initializations in proc_alloc_inode()
    vfs: Add current_time() api
    vfs: add note about i_op->rename changes to porting
    fs: rename "rename2" i_op to "rename"
    vfs: remove unused i_op->rename
    fs: make remaining filesystems use .rename2
    libfs: support RENAME_NOREPLACE in simple_rename()
    fs: support RENAME_NOREPLACE for local filesystems
    ncpfs: fix unused variable warning

    Linus Torvalds
     
  • Pull vfs xattr updates from Al Viro:
    "xattr stuff from Andreas

    This completes the switch to xattr_handler ->get()/->set() from
    ->getxattr/->setxattr/->removexattr"

    * 'work.xattr' of git://git.kernel.org/pub/scm/linux/kernel/git/viro/vfs:
    vfs: Remove {get,set,remove}xattr inode operations
    xattr: Stop calling {get,set,remove}xattr inode operations
    vfs: Check for the IOP_XATTR flag in listxattr
    xattr: Add __vfs_{get,set,remove}xattr helpers
    libfs: Use IOP_XATTR flag for empty directory handling
    vfs: Use IOP_XATTR flag for bad-inode handling
    vfs: Add IOP_XATTR inode operations flag
    vfs: Move xattr_resolve_name to the front of fs/xattr.c
    ecryptfs: Switch to generic xattr handlers
    sockfs: Get rid of getxattr iop
    sockfs: getxattr: Fail with -EOPNOTSUPP for invalid attribute names
    kernfs: Switch to generic xattr handlers
    hfs: Switch to generic xattr handlers
    jffs2: Remove jffs2_{get,set,remove}xattr macros
    xattr: Remove unnecessary NULL attribute name check

    Linus Torvalds
     

08 Oct, 2016

1 commit

  • Right now, various places in the kernel check for the existence of
    getxattr, setxattr, and removexattr inode operations and directly call
    those operations. Switch to helper functions and test for the IOP_XATTR
    flag instead.

    Signed-off-by: Andreas Gruenbacher
    Acked-by: James Morris
    Signed-off-by: Al Viro

    Andreas Gruenbacher
     

28 Sep, 2016

1 commit

  • An NULL-pointer dereference happens in cachefiles_mark_object_inactive()
    when it tries to read i_blocks so that it can tell the cachefilesd daemon
    how much space it's making available.

    The problem is that cachefiles_drop_object() calls
    cachefiles_mark_object_inactive() after calling cachefiles_delete_object()
    because the object being marked active staves off attempts to (re-)use the
    file at that filename until after it has been deleted. This means that
    d_inode is NULL by the time we come to try to access it.

    To fix the problem, have the caller of cachefiles_mark_object_inactive()
    supply the number of blocks freed up.

    Without this, the following oops may occur:

    BUG: unable to handle kernel NULL pointer dereference at 0000000000000098
    IP: [] cachefiles_mark_object_inactive+0x61/0xb0 [cachefiles]
    ...
    CPU: 11 PID: 527 Comm: kworker/u64:4 Tainted: G I ------------ 3.10.0-470.el7.x86_64 #1
    Hardware name: Hewlett-Packard HP Z600 Workstation/0B54h, BIOS 786G4 v03.19 03/11/2011
    Workqueue: fscache_object fscache_object_work_func [fscache]
    task: ffff880035edaf10 ti: ffff8800b77c0000 task.ti: ffff8800b77c0000
    RIP: 0010:[] cachefiles_mark_object_inactive+0x61/0xb0 [cachefiles]
    RSP: 0018:ffff8800b77c3d70 EFLAGS: 00010246
    RAX: 0000000000000000 RBX: ffff8800bf6cc400 RCX: 0000000000000034
    RDX: 0000000000000000 RSI: ffff880090ffc710 RDI: ffff8800bf761ef8
    RBP: ffff8800b77c3d88 R08: 2000000000000000 R09: 0090ffc710000000
    R10: ff51005d2ff1c400 R11: 0000000000000000 R12: ffff880090ffc600
    R13: ffff8800bf6cc520 R14: ffff8800bf6cc400 R15: ffff8800bf6cc498
    FS: 0000000000000000(0000) GS:ffff8800bb8c0000(0000) knlGS:0000000000000000
    CS: 0010 DS: 0000 ES: 0000 CR0: 000000008005003b
    CR2: 0000000000000098 CR3: 00000000019ba000 CR4: 00000000000007e0
    DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
    DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
    Stack:
    ffff880090ffc600 ffff8800bf6cc400 ffff8800867df140 ffff8800b77c3db0
    ffffffffa06c48cb ffff880090ffc600 ffff880090ffc180 ffff880090ffc658
    ffff8800b77c3df0 ffffffffa085d846 ffff8800a96b8150 ffff880090ffc600
    Call Trace:
    [] cachefiles_drop_object+0x6b/0xf0 [cachefiles]
    [] fscache_drop_object+0xd6/0x1e0 [fscache]
    [] fscache_object_work_func+0xa5/0x200 [fscache]
    [] process_one_work+0x17b/0x470
    [] worker_thread+0x126/0x410
    [] ? rescuer_thread+0x460/0x460
    [] kthread+0xcf/0xe0
    [] ? kthread_create_on_node+0x140/0x140
    [] ret_from_fork+0x58/0x90
    [] ? kthread_create_on_node+0x140/0x140

    The oopsing code shows:

    callq 0xffffffff810af6a0
    mov 0xf8(%r12),%rax
    mov 0x30(%rax),%rax
    mov 0x98(%rax),%rax dentry)->i_blocks

    Fixes: a5b3a80b899bda0f456f1246c4c5a1191ea01519 (CacheFiles: Provide read-and-reset release counters for cachefilesd)
    Reported-by: Jianhong Yin
    Signed-off-by: David Howells
    Reviewed-by: Jeff Layton
    Reviewed-by: Steve Dickson
    cc: stable@vger.kernel.org
    Signed-off-by: Al Viro

    David Howells
     

27 Sep, 2016

2 commits


04 Aug, 2016

1 commit

  • There's a race between cachefiles_mark_object_inactive() and
    cachefiles_cull():

    (1) cachefiles_cull() can't delete a backing file until the cache object
    is marked inactive, but as soon as that's the case it's fair game.

    (2) cachefiles_mark_object_inactive() marks the object as being inactive
    and *only then* reads the i_blocks on the backing inode - but
    cachefiles_cull() might've managed to delete it by this point.

    Fix this by making sure cachefiles_mark_object_inactive() gets any data it
    needs from the backing inode before deactivating the object.

    Without this, the following oops may occur:

    BUG: unable to handle kernel NULL pointer dereference at 0000000000000098
    IP: [] cachefiles_mark_object_inactive+0x61/0xb0 [cachefiles]
    ...
    CPU: 11 PID: 527 Comm: kworker/u64:4 Tainted: G I ------------ 3.10.0-470.el7.x86_64 #1
    Hardware name: Hewlett-Packard HP Z600 Workstation/0B54h, BIOS 786G4 v03.19 03/11/2011
    Workqueue: fscache_object fscache_object_work_func [fscache]
    task: ffff880035edaf10 ti: ffff8800b77c0000 task.ti: ffff8800b77c0000
    RIP: 0010:[] cachefiles_mark_object_inactive+0x61/0xb0 [cachefiles]
    RSP: 0018:ffff8800b77c3d70 EFLAGS: 00010246
    RAX: 0000000000000000 RBX: ffff8800bf6cc400 RCX: 0000000000000034
    RDX: 0000000000000000 RSI: ffff880090ffc710 RDI: ffff8800bf761ef8
    RBP: ffff8800b77c3d88 R08: 2000000000000000 R09: 0090ffc710000000
    R10: ff51005d2ff1c400 R11: 0000000000000000 R12: ffff880090ffc600
    R13: ffff8800bf6cc520 R14: ffff8800bf6cc400 R15: ffff8800bf6cc498
    FS: 0000000000000000(0000) GS:ffff8800bb8c0000(0000) knlGS:0000000000000000
    CS: 0010 DS: 0000 ES: 0000 CR0: 000000008005003b
    CR2: 0000000000000098 CR3: 00000000019ba000 CR4: 00000000000007e0
    DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
    DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
    Stack:
    ffff880090ffc600 ffff8800bf6cc400 ffff8800867df140 ffff8800b77c3db0
    ffffffffa06c48cb ffff880090ffc600 ffff880090ffc180 ffff880090ffc658
    ffff8800b77c3df0 ffffffffa085d846 ffff8800a96b8150 ffff880090ffc600
    Call Trace:
    [] cachefiles_drop_object+0x6b/0xf0 [cachefiles]
    [] fscache_drop_object+0xd6/0x1e0 [fscache]
    [] fscache_object_work_func+0xa5/0x200 [fscache]
    [] process_one_work+0x17b/0x470
    [] worker_thread+0x126/0x410
    [] ? rescuer_thread+0x460/0x460
    [] kthread+0xcf/0xe0
    [] ? kthread_create_on_node+0x140/0x140
    [] ret_from_fork+0x58/0x90
    [] ? kthread_create_on_node+0x140/0x140

    The oopsing code shows:

    callq 0xffffffff810af6a0
    mov 0xf8(%r12),%rax
    mov 0x30(%rax),%rax
    mov 0x98(%rax),%rax dentry)->i_blocks

    Fixes: a5b3a80b899bda0f456f1246c4c5a1191ea01519 (CacheFiles: Provide read-and-reset release counters for cachefilesd)
    Reported-by: Jianhong Yin
    Signed-off-by: David Howells
    Reviewed-by: Jeff Layton
    Reviewed-by: Steve Dickson
    cc: stable@vger.kernel.org
    Signed-off-by: Al Viro

    David Howells
     

01 Jul, 2016

1 commit


01 Jun, 2016

1 commit

  • __fscache_check_consistency() calls check_consistency() callback
    and return the callback's return value. But the return type of
    check_consistency() is bool. So __fscache_check_consistency()
    return 1 if the cache is inconsistent. This is inconsistent with
    the document.

    Signed-off-by: Yan, Zheng
    Acked-by: David Howells

    Yan, Zheng
     

30 May, 2016

1 commit


05 Apr, 2016

1 commit

  • PAGE_CACHE_{SIZE,SHIFT,MASK,ALIGN} macros were introduced *long* time
    ago with promise that one day it will be possible to implement page
    cache with bigger chunks than PAGE_SIZE.

    This promise never materialized. And unlikely will.

    We have many places where PAGE_CACHE_SIZE assumed to be equal to
    PAGE_SIZE. And it's constant source of confusion on whether
    PAGE_CACHE_* or PAGE_* constant should be used in a particular case,
    especially on the border between fs and mm.

    Global switching to PAGE_CACHE_SIZE != PAGE_SIZE would cause to much
    breakage to be doable.

    Let's stop pretending that pages in page cache are special. They are
    not.

    The changes are pretty straight-forward:

    - << (PAGE_CACHE_SHIFT - PAGE_SHIFT) -> ;

    - >> (PAGE_CACHE_SHIFT - PAGE_SHIFT) -> ;

    - PAGE_CACHE_{SIZE,SHIFT,MASK,ALIGN} -> PAGE_{SIZE,SHIFT,MASK,ALIGN};

    - page_cache_get() -> get_page();

    - page_cache_release() -> put_page();

    This patch contains automated changes generated with coccinelle using
    script below. For some reason, coccinelle doesn't patch header files.
    I've called spatch for them manually.

    The only adjustment after coccinelle is revert of changes to
    PAGE_CAHCE_ALIGN definition: we are going to drop it later.

    There are few places in the code where coccinelle didn't reach. I'll
    fix them manually in a separate patch. Comments and documentation also
    will be addressed with the separate patch.

    virtual patch

    @@
    expression E;
    @@
    - E << (PAGE_CACHE_SHIFT - PAGE_SHIFT)
    + E

    @@
    expression E;
    @@
    - E >> (PAGE_CACHE_SHIFT - PAGE_SHIFT)
    + E

    @@
    @@
    - PAGE_CACHE_SHIFT
    + PAGE_SHIFT

    @@
    @@
    - PAGE_CACHE_SIZE
    + PAGE_SIZE

    @@
    @@
    - PAGE_CACHE_MASK
    + PAGE_MASK

    @@
    expression E;
    @@
    - PAGE_CACHE_ALIGN(E)
    + PAGE_ALIGN(E)

    @@
    expression E;
    @@
    - page_cache_get(E)
    + get_page(E)

    @@
    expression E;
    @@
    - page_cache_release(E)
    + put_page(E)

    Signed-off-by: Kirill A. Shutemov
    Acked-by: Michal Hocko
    Signed-off-by: Linus Torvalds

    Kirill A. Shutemov
     

02 Feb, 2016

1 commit

  • Provide read-and-reset objects- and blocks-released counters for cachefilesd
    to use to work out whether there's anything new that can be culled.

    One of the problems cachefilesd has is that if all the objects in the cache
    are pinned by inodes lying dormant in the kernel inode cache, there isn't
    anything for it to cull. In such a case, it just spins around walking the
    filesystem tree and scanning for something to cull. This eats up a lot of
    CPU time.

    By telling cachefilesd if there have been any releases, the daemon can
    sleep until there is the possibility of something to do.

    cachefilesd finds this information by the following means:

    (1) When the control fd is read, the kernel presents a list of values of
    interest. "freleased=N" and "breleased=N" are added to this list to
    indicate the number of files released and number of blocks released
    since the last read call. At this point the counters are reset.

    (2) POLLIN is signalled if the number of files released becomes greater
    than 0.

    Note that by 'released' it just means that the kernel has released its
    interest in those files for the moment, not necessarily that the files
    should be deleted from the cache.

    Signed-off-by: David Howells
    Reviewed-by: Steve Dickson
    Signed-off-by: Al Viro

    David Howells
     

23 Jan, 2016

1 commit

  • parallel to mutex_{lock,unlock,trylock,is_locked,lock_nested},
    inode_foo(inode) being mutex_foo(&inode->i_mutex).

    Please, use those for access to ->i_mutex; over the coming cycle
    ->i_mutex will become rwsem, with ->lookup() done with it held
    only shared.

    Signed-off-by: Al Viro

    Al Viro
     

04 Jan, 2016

1 commit


17 Nov, 2015

1 commit

  • fs/cachefiles/rdwr.c: In function ‘cachefiles_write_page’:
    fs/cachefiles/rdwr.c:882: warning: ‘ret’ may be used uninitialized in
    this function

    If the jump to label "error" is taken, "ret" will indeed be
    uninitialized, and random stack data may be printed by the debug code.

    Fixes: 102f4d900c9c8f5e ("FS-Cache: Handle a write to the page immediately beyond the EOF marker")
    Signed-off-by: Geert Uytterhoeven
    Signed-off-by: David Howells
    Signed-off-by: Al Viro

    Geert Uytterhoeven
     

11 Nov, 2015

2 commits

  • Handle a write being requested to the page immediately beyond the EOF
    marker on a cache object. Currently this gets an assertion failure in
    CacheFiles because the EOF marker is used there to encode information about
    a partial page at the EOF - which could lead to an unknown blank spot in
    the file if we extend the file over it.

    The problem is actually in fscache where we check the index of the page
    being written against store_limit. store_limit is set to the number of
    pages that we're allowed to store by fscache_set_store_limit() - which
    means it's one more than the index of the last page we're allowed to store.
    The problem is that we permit writing to a page with an index _equal_ to
    the store limit - when we should reject that case.

    Whilst we're at it, change the triggered assertion in CacheFiles to just
    return -ENOBUFS instead.

    The assertion failure looks something like this:

    CacheFiles: Assertion failed
    1000 < 7b1 is false
    ------------[ cut here ]------------
    kernel BUG at fs/cachefiles/rdwr.c:962!
    ...
    RIP: 0010:[] [] cachefiles_write_page+0x273/0x2d0 [cachefiles]

    Cc: stable@vger.kernel.org # v2.6.31+; earlier - that + backport of a17754f (at least)
    Signed-off-by: David Howells
    Signed-off-by: Al Viro

    David Howells
     
  • cachefiles requires that s_blocksize in the cache is not greater than
    PAGE_SIZE, and performs the check every time a block is accessed.

    Move the test to the place where the file is "opened", where other
    file-validity tests are performed.

    Signed-off-by: NeilBrown
    Signed-off-by: David Howells
    Signed-off-by: Al Viro

    NeilBrown
     

07 Nov, 2015

1 commit

  • __GFP_WAIT was used to signal that the caller was in atomic context and
    could not sleep. Now it is possible to distinguish between true atomic
    context and callers that are not willing to sleep. The latter should
    clear __GFP_DIRECT_RECLAIM so kswapd will still wake. As clearing
    __GFP_WAIT behaves differently, there is a risk that people will clear the
    wrong flags. This patch renames __GFP_WAIT to __GFP_RECLAIM to clearly
    indicate what it does -- setting it allows all reclaim activity, clearing
    them prevents it.

    [akpm@linux-foundation.org: fix build]
    [akpm@linux-foundation.org: coding-style fixes]
    Signed-off-by: Mel Gorman
    Acked-by: Michal Hocko
    Acked-by: Vlastimil Babka
    Acked-by: Johannes Weiner
    Cc: Christoph Lameter
    Acked-by: David Rientjes
    Cc: Vitaly Wool
    Cc: Rik van Riel
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    Mel Gorman
     

24 Jun, 2015

1 commit


16 Apr, 2015

2 commits


24 Feb, 2015

1 commit


23 Feb, 2015

2 commits

  • Fix up the following scripted S_ISDIR/S_ISREG/S_ISLNK conversions (or lack
    thereof) in cachefiles:

    (1) Cachefiles mostly wants to use d_can_lookup() rather than d_is_dir() as
    it doesn't want to deal with automounts in its cache.

    (2) Coccinelle didn't find S_IS* expressions in ASSERT() statements in
    cachefiles.

    Signed-off-by: David Howells
    Signed-off-by: Al Viro

    David Howells
     
  • Convert the following where appropriate:

    (1) S_ISLNK(dentry->d_inode) to d_is_symlink(dentry).

    (2) S_ISREG(dentry->d_inode) to d_is_reg(dentry).

    (3) S_ISDIR(dentry->d_inode) to d_is_dir(dentry). This is actually more
    complicated than it appears as some calls should be converted to
    d_can_lookup() instead. The difference is whether the directory in
    question is a real dir with a ->lookup op or whether it's a fake dir with
    a ->d_automount op.

    In some circumstances, we can subsume checks for dentry->d_inode not being
    NULL into this, provided we the code isn't in a filesystem that expects
    d_inode to be NULL if the dirent really *is* negative (ie. if we're going to
    use d_inode() rather than d_backing_inode() to get the inode pointer).

    Note that the dentry type field may be set to something other than
    DCACHE_MISS_TYPE when d_inode is NULL in the case of unionmount, where the VFS
    manages the fall-through from a negative dentry to a lower layer. In such a
    case, the dentry type of the negative union dentry is set to the same as the
    type of the lower dentry.

    However, if you know d_inode is not NULL at the call site, then you can use
    the d_is_xxx() functions even in a filesystem.

    There is one further complication: a 0,0 chardev dentry may be labelled
    DCACHE_WHITEOUT_TYPE rather than DCACHE_SPECIAL_TYPE. Strictly, this was
    intended for special directory entry types that don't have attached inodes.

    The following perl+coccinelle script was used:

    use strict;

    my @callers;
    open($fd, 'git grep -l \'S_IS[A-Z].*->d_inode\' |') ||
    die "Can't grep for S_ISDIR and co. callers";
    @callers = ;
    close($fd);
    unless (@callers) {
    print "No matches\n";
    exit(0);
    }

    my @cocci = (
    '@@',
    'expression E;',
    '@@',
    '',
    '- S_ISLNK(E->d_inode->i_mode)',
    '+ d_is_symlink(E)',
    '',
    '@@',
    'expression E;',
    '@@',
    '',
    '- S_ISDIR(E->d_inode->i_mode)',
    '+ d_is_dir(E)',
    '',
    '@@',
    'expression E;',
    '@@',
    '',
    '- S_ISREG(E->d_inode->i_mode)',
    '+ d_is_reg(E)' );

    my $coccifile = "tmp.sp.cocci";
    open($fd, ">$coccifile") || die $coccifile;
    print($fd "$_\n") || die $coccifile foreach (@cocci);
    close($fd);

    foreach my $file (@callers) {
    chomp $file;
    print "Processing ", $file, "\n";
    system("spatch", "--sp-file", $coccifile, $file, "--in-place", "--no-show-diff") == 0 ||
    die "spatch failed";
    }

    [AV: overlayfs parts skipped]

    Signed-off-by: David Howells
    Signed-off-by: Al Viro

    David Howells
     

20 Nov, 2014

1 commit


14 Oct, 2014

2 commits

  • …git/dhowells/linux-fs

    Pull fs-cache fixes from David Howells:
    "Two fixes for bugs in CacheFiles and a cleanup in FS-Cache"

    * tag 'fscache-fixes-20141013' of git://git.kernel.org/pub/scm/linux/kernel/git/dhowells/linux-fs:
    fs/fscache/object-list.c: use __seq_open_private()
    CacheFiles: Fix incorrect test for in-memory object collision
    CacheFiles: Handle object being killed before being set up

    Linus Torvalds
     
  • When CacheFiles cache objects are in use, they have in-memory representations,
    as defined by the cachefiles_object struct. These are kept in a tree rooted in
    the cache and indexed by dentry pointer (since there's a unique mapping between
    object index key and dentry).

    Collisions can occur between a representation already in the tree and a new
    representation being set up because it takes time to dispose of an old
    representation - particularly if it must be unlinked or renamed.

    When such a collision occurs, cachefiles_mark_object_active() is meant to check
    to see if the old, already-present representation is in the process of being
    discarded (ie. FSCACHE_OBJECT_IS_LIVE is not set on it) - and, if so, wait for
    the representation to be removed (ie. CACHEFILES_OBJECT_ACTIVE is then
    cleared).

    However, the test for whether the old representation is still live is checking
    the new object - which always will be live at this point. This leads to an
    oops looking like:

    CacheFiles: Error: Unexpected object collision
    object: OBJ1b354
    objstate=LOOK_UP_OBJECT fl=8 wbusy=2 ev=0[0]
    ops=0 inp=0 exc=0
    parent=ffff88053f5417c0
    cookie=ffff880538f202a0 [pr=ffff8805381b7160 nd=ffff880509c6eb78 fl=27]
    key=[8] '2490000000000000'
    xobject: OBJ1a600
    xobjstate=DROP_OBJECT fl=70 wbusy=2 ev=0[0]
    xops=0 inp=0 exc=0
    xparent=ffff88053f5417c0
    xcookie=ffff88050f4cbf70 [pr=ffff8805381b7160 nd= (null) fl=12]
    ------------[ cut here ]------------
    kernel BUG at fs/cachefiles/namei.c:200!
    ...
    Workqueue: fscache_object fscache_object_work_func [fscache]
    ...
    RIP: ... cachefiles_walk_to_object+0x7ea/0x860 [cachefiles]
    ...
    Call Trace:
    [] ? cachefiles_lookup_object+0x58/0x100 [cachefiles]
    [] ? fscache_look_up_object+0xb9/0x1d0 [fscache]
    [] ? fscache_parent_ready+0x2d/0x80 [fscache]
    [] ? fscache_object_work_func+0x92/0x1f0 [fscache]
    [] ? process_one_work+0x16b/0x400
    [] ? worker_thread+0x116/0x380
    [] ? manage_workers.isra.21+0x290/0x290
    [] ? kthread+0xbc/0xe0
    [] ? flush_kthread_worker+0x80/0x80
    [] ? ret_from_fork+0x7c/0xb0
    [] ? flush_kthread_worker+0x80/0x80

    Reported-by: Manuel Schölling
    Signed-off-by: David Howells
    Acked-by: Steve Dickson

    David Howells
     

09 Oct, 2014

1 commit


30 Sep, 2014

1 commit

  • If a cache object gets killed whilst in the process of being set up - for
    instance if the netfs relinquishes the cookie that the object is associated
    with - then the object's state machine will transit to the DROP_OBJECT state
    without necessarily going through the LOOKUP_OBJECT or CREATE_OBJECT states.

    This is a problem for CacheFiles because cachefiles_drop_object() assumes that
    object->dentry will be set upon reaching the DROP_OBJECT state and has an
    ASSERT() to that effect (see the oops below) - but object->dentry doesn't get
    set until the LOOKUP_OBJECT or CREATE_OBJECT states (and not always then if
    they fail).

    To fix this, just make the dentry cleanup in cachefiles_drop_object()
    conditional on the dentry actually being set and remove the assertion.

    CacheFiles: Assertion failed
    ------------[ cut here ]------------
    kernel BUG at .../fs/cachefiles/namei.c:425!
    ...
    Workqueue: fscache_object fscache_object_work_func [fscache]
    ...
    RIP: ... cachefiles_delete_object+0xcd/0x110 [cachefiles]
    ...
    Call Trace:
    [] ? cachefiles_drop_object+0xff/0x130 [cachefiles]
    [] ? fscache_drop_object+0xd1/0x1d0 [fscache]
    [] ? fscache_object_work_func+0x87/0x210 [fscache]
    [] ? process_one_work+0x155/0x450
    [] ? worker_thread+0x114/0x370
    [] ? manage_workers.isra.21+0x2c0/0x2c0
    [] ? kthread+0xbc/0xe0
    [] ? flush_kthread_worker+0xa0/0xa0
    [] ? ret_from_fork+0x7c/0xb0
    [] ? flush_kthread_worker+0xa0/0xa0

    Reported-by: Manuel Schölling
    Signed-off-by: David Howells
    Acked-by: Steve Dickson

    David Howells
     

26 Sep, 2014

1 commit


18 Sep, 2014

2 commits

  • Not all filesystems now provide the rename i_op - ext4 for one - but rather
    provide the rename2 i_op. CacheFiles checks that the filesystem has rename
    and so will reject ext4 now with EPERM:

    CacheFiles: Failed to register: -1

    Fix this by checking for rename2 as an alternative. The call to vfs_rename()
    actually handles selection of the appropriate function, so we needn't worry
    about that.

    Turning on debugging shows:

    [cachef] ==> cachefiles_get_directory(,,cache)
    [cachef] subdir -> ffff88000b22b778 positive
    [cachef]

    David Howells
     
  • These two have been unused since

    commit c4d6d8dbf335c7fa47341654a37c53a512b519bb
    CacheFiles: Fix the marking of cached pages

    in 3.8.

    Signed-off-by: NeilBrown
    Signed-off-by: David Howells

    NeilBrown