Bugzilla (bugzilla.redhat.com) will be under maintenance for infrastructure upgrades and will not be available on July 31st between 12:30 AM - 05:30 AM UTC. We appreciate your understanding and patience. You can follow status.redhat.com for details.
Bug 818588 - knfsd: encapsulate the share/deny mode routines
Summary: knfsd: encapsulate the share/deny mode routines
Keywords:
Status: CLOSED UPSTREAM
Alias: None
Product: Fedora
Classification: Fedora
Component: kernel
Version: 19
Hardware: Unspecified
OS: Unspecified
low
low
Target Milestone: ---
Assignee: Jeff Layton
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2012-05-03 13:05 UTC by Jeff Layton
Modified: 2014-06-18 07:42 UTC (History)
7 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2013-04-10 10:48:44 UTC
Type: Bug


Attachments (Terms of Use)
locks - begin enforcing LOCK_MAND locks by default (3.13 KB, patch)
2012-07-06 20:11 UTC, Jeff Layton
no flags Details | Diff

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.


Note You need to log in before you can comment on or make changes to this bug.