Bug 818588

Summary: knfsd: encapsulate the share/deny mode routines
Product: [Fedora] Fedora Reporter: Jeff Layton <jlayton>
Component: kernelAssignee: Jeff Layton <jlayton>
Status: CLOSED UPSTREAM QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: low Docs Contact:
Priority: low    
Version: 19CC: bfields, gansalmon, itamar, jonathan, kernel-maint, madhu.chinakonda, steved
Target Milestone: ---   
Target Release: ---   
Hardware: Unspecified   
OS: Unspecified   
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2013-04-10 10:48:44 UTC Type: Bug
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:
Attachments:
Description Flags
locks - begin enforcing LOCK_MAND locks by default none

Description Jeff Layton 2012-05-03 13:05:14 UTC
I just need a place to track and keep notes on this project for upstream
work, so here it is.

In preparation of the clustering work for knfsd, we'll need to be able to
swap in a different set of routines to handle tracking share/deny mode stuff.
We may also want to eventually switch to using the flock() LOCK_MAND code
to handle this, and that would make it easier to ensure that we're doing
the right thing.

Comment 1 Jeff Layton 2012-07-05 14:59:39 UTC
I sent a set of patches to Bruce about a month ago that encapsulates the internal knfsd share reservation code by adding accessor methods for it. That went in for 3.5, but is just a cleanup set -- it doesn't really offer any behavior changes.

I've also been looking over the existing LOCK_MAND code in Linux. We might be able to beat that into something workable here, but it's pretty useless in its current form.

The idea with the existing code is that a process would universally set a flock(..., LOCK_MAND) lock after opening a file. If the task wants to allow others to read the file, then they'd set LOCK_READ. Ditto for LOCK_WRITE for users writing to it. LOCK_MAND|LOCK_READ|LOCK_WRITE is therefore the same as not setting a lock at all, AFAICT.

There are some problems with this interface:

1) it's racy: we could really stand to have this be atomic with open(), but that's probably not doable initially

2) the kernel does no enforcement of these locks whatsoever. It appears to be expected that knfsd will call lock_may_read and lock_may_write to enforce these. Nothing calls those functions however. Rumor has it that GPFS does do some enforcement here, which is why samba calls this stuff, but no open source filesystem that I can find actually does.

3) because these are flock() locks, there is no mechanism to query them, so userspace can't enforce them.

Given the above, I sort of feel like we have carte blanche to change this into a more usable interface. The question is what semantics we want. We will probably also want to give the GPFS people a heads up to ensure that our changes don't break them too badly.

Comment 2 Jeff Layton 2012-07-06 20:11:17 UTC
Created attachment 596682 [details]
locks - begin enforcing LOCK_MAND locks by default

So, here's a relatively simple (but experimental!) patch to add some teeth to LOCK_MAND. It's still pretty klunky so I imagine some pushback. It is unlikely to break anyone that doesn't want to use it however...

Share reservations with LOCK_MAND are only enforced against filps with LOCK_MAND locks on them, but we check against the f_mode of the associated filp.

The upshot is that applications like knfsd or samba will need to set a LOCK_MAND lock on every filp they open. When they don't want to do a DENY_NONE type reservation, then they'll use (LOCK_MAND|LOCK_READ|LOCK_WRITE).

All LOCK_MAND locks are considered to be non-blocking as well in this patch, though we could reconsider that at some point.

The patch basically works when I test it with a simple userspace test program that I wrote.

This is unlikely to break samba (since it tracks enforcement of this stuff internally), but if it wanted to be use it, samba would need to be changed to set a (LOCK_MAND|LOCK_READ|LOCK_WRITE) reservation on DENY_NONE opens.

It's unclear to me how well these sematics jive with the rumored GPFS support for this.

Comment 3 Jeff Layton 2013-03-14 11:45:11 UTC
Pavel Shilovsky has picked up this work upstream and is working on a solution. At this point, I'll let him drive it but do plan to help review it.

Comment 4 Fedora End Of Life 2013-04-03 20:06:04 UTC
This bug appears to have been reported against 'rawhide' during the Fedora 19 development cycle.
Changing version to '19'.

(As we did not run this process for some time, it could affect also pre-Fedora 19 development
cycle bugs. We are very sorry. It will help us with cleanup during Fedora 19 End Of Life. Thank you.)

More information and reason for this action is here:
https://fedoraproject.org/wiki/BugZappers/HouseKeeping/Fedora19

Comment 5 Jeff Layton 2013-04-10 10:48:44 UTC
Pavel Shilovsky has taken this on upstream for now and I'm helping to review it. Closing with a resolution of UPSTREAM.