Re: [PATCH 0/3] Include OFD lock owners when looking up state

[Benjamin Coddington <bcodding@redhat.com>]

kbuild robot noticed I made a mistake doing a quick rebase.  I'll send a V2
for completeness even if it is disliked.  I hope we can keep discussing
it..

Ben

On Fri, 1 Apr 2016, Benjamin Coddington wrote:

> The client sends IO only under the open stateid when using OFD (and flock)
> locking instead of the appropriate lock stateid because the nfs_lock_context
> only tracks POSIX lockowners, which is the reference to the process' file
> table.
>
> This is a problem for two reasons.  The first is that rfc7530,
> section-9.1.4.5 states that IO sent by an entity corresponding to the
> lock-owner which holds a byte-range lock should be sent under the lock
> stateid for that lock.  Otherwise, a server enforcing mandatory byte-range
> locking might reject that operation.  Secondly, not tracking OFD lock owners
> means that accounting for IO sent under those owners is broken.  That
> creates a problem for some work to guarantee an unlock will be sent after
> operations scheduled under a lock complete.
>
> To fix this, this set starts tracking the OFD lock owner within the
> nfs_lock_context.  Unique nfs_lock_contexts are created on distinct
> combinations of current->files, struct file, and pid.  In order to do this,
> some re-arrangement of when to create or lookup the nfs_lock_context was
> needed since the struct file pointer must be included during that creation
> or lookup.
>
> It is possible for a process to hold both an OFD and a POSIX lock on a file
> as long as they do not conflict.  There would be two stateids that could be
> selected for IO operations on that file.  In that case only the first
> stateid found to match the current lock context will ever be used.  The
> result of this is that mixed OFD and POSIX locks within the same process on
> a server enforcing mandatory locking should be avoided.  The fix for this
> probably would require a byte-range lookup of stateid when scheduling an IO
> operation, and the ability to split IO that crossed lock ranges with
> different owners.
>
> Comments and critique is welcomed.
>
> Benjamin Coddington (3):
>   NFS: add get_nfs_lock_context, find_nfs_lock_context
>   NFS: add OFD lock owners to nfs_lock_context
>   NFSv4: use OFD lock owners in lock state lookup
>
>  fs/nfs/direct.c          |    8 ++++----
>  fs/nfs/file.c            |    2 +-
>  fs/nfs/fscache-index.c   |    4 ++--
>  fs/nfs/fscache.h         |   12 ++++++------
>  fs/nfs/inode.c           |   32 ++++++++++++++++++++++----------
>  fs/nfs/nfs42proc.c       |    8 ++++----
>  fs/nfs/nfs4proc.c        |    3 ++-
>  fs/nfs/nfs4state.c       |   21 ++++++++++++---------
>  fs/nfs/pagelist.c        |   22 ++++++++--------------
>  fs/nfs/read.c            |   38 +++++++++++++++++++++++---------------
>  fs/nfs/write.c           |   17 ++++++++++-------
>  include/linux/nfs_fs.h   |    8 +++++---
>  include/linux/nfs_page.h |    2 +-
>  13 files changed, 100 insertions(+), 77 deletions(-)
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>