2011-11-29 09:12:20

by Stanislav Kinsbursky

[permalink] [raw]
Subject: [PATCH 0/5] NFS: create blocklayout pipe per network namesapce context

This patch set was created in context of clone of git
branch: git://git.linux-nfs.org/projects/trondmy/nfs-2.6.git.
tag: v3.1

This patch set depends on previous patch sets titled:
1) "SUNRPC: initial part of making pipefs work in net ns"
2) "SUNPRC: cleanup PipeFS for network-namespace-aware users"
3) "SUNRPC: make RPC clients use network-namespace-aware PipeFS routines"
4) "NFS: create clients and IDMAP pipes per network namespace"

This patch set is the final part of making SUNRPC PipeFS and it's users work in
network namespace context.

The following series consists of:

---

Stanislav Kinsbursky (5):
NFS: handle blocklayout pipe PipeFS dentry by network namespace aware routines
NFS: blocklayout pipe creation per network namespace context introduced
NFS: blocklayout PipeFS notifier introduced
NFS: remove RPC PipeFS mount point reference from blocklayout routines
SUNRPC: kernel PipeFS mount point creation routines removed


fs/nfs/blocklayout/blocklayout.c | 154 ++++++++++++++++++++++++++++-------
fs/nfs/blocklayout/blocklayout.h | 3 -
fs/nfs/blocklayout/blocklayoutdev.c | 5 +
fs/nfs/blocklayout/blocklayoutdm.c | 7 +-
fs/nfs/inode.c | 1
fs/nfs/netns.h | 1
include/linux/sunrpc/rpc_pipe_fs.h | 2
net/sunrpc/rpc_pipe.c | 21 -----
8 files changed, 137 insertions(+), 57 deletions(-)



2011-11-29 15:30:44

by Peng Tao

[permalink] [raw]
Subject: Re: [PATCH 4/5] NFS: remove RPC PipeFS mount point reference from blocklayout routines

On Tue, Nov 29, 2011 at 11:18 PM, Trond Myklebust
<[email protected]> wrote:
> On Tue, 2011-11-29 at 23:10 +0800, Peng Tao wrote:
>> On Tue, Nov 29, 2011 at 9:35 PM, Myklebust, Trond
>> <[email protected]> wrote:
>> >> -----Original Message-----
>> >> From: [email protected] [mailto:[email protected]]
>> >> Sent: Tuesday, November 29, 2011 7:40 AM
>> >> To: [email protected]
>> >> Cc: Myklebust, Trond; [email protected]; [email protected];
>> >> [email protected]; [email protected]; [email protected];
>> >> [email protected]; [email protected]; [email protected];
>> >> [email protected]
>> >> Subject: RE: [PATCH 4/5] NFS: remove RPC PipeFS mount point reference
>> >> from blocklayout routines
>> >>
>> >> > -----Original Message-----
>> >> > From: Stanislav Kinsbursky [mailto:[email protected]]
>> >> > Sent: Tuesday, November 29, 2011 8:19 PM
>> >> > To: Peng, Tao
>> >> > Cc: [email protected]; [email protected]; Pavel
>> >> > Emelianov; [email protected]; [email protected];
>> >> > [email protected]; James Bottomley; [email protected];
>> >> > [email protected]; [email protected]
>> >> > Subject: Re: [PATCH 4/5] NFS: remove RPC PipeFS mount point reference
>> >> > from blocklayout routines
>> >> >
>> >> > 29.11.2011 16:00, [email protected] пишет:
>> >> > >> -----Original Message-----
>> >> > >> From: [email protected]
>> >> > >> [mailto:[email protected]] On Behalf Of
>> >> > Stanislav
>> >> > >> Kinsbursky
>> >> > >> Sent: Tuesday, November 29, 2011 6:11 PM
>> >> > >> To: [email protected]
>> >> > >> Cc: [email protected]; [email protected]; [email protected];
>> >> > >> [email protected]; linux- [email protected];
>> >> > >> [email protected]; [email protected];
>> >> > >> [email protected]; [email protected]
>> >> > >> Subject: [PATCH 4/5] NFS: remove RPC PipeFS mount point reference
>> >> > >> from blocklayout routines
>> >> > >>
>> >> > >> This is a cleanup patch. We don't need this reference anymore,
>> >> > >> because blocklayout pipes dentries now creates and destroys in
>> >> > >> per-net operations and on PipeFS mount/umount notification.
>> >> > >> Note that nfs4blocklayout_register_net() now returns 0 instead of
>> >> > >> -ENOENT in case of PipeFS superblock absence. This is ok, because
>> >> > >> blocklayout pipe dentry will be created on PipeFS mount event.
>> >> > > When is the "pipefs mount event" going to happen? When inserting
>> >> > > kernel modules or when user issues
>> >> > mount command?
>> >> > >
>> >> >
>> >> > When user issues mount command.
>> >> > Kernel mounts of PipeFS has been removed with all these patch sets
>> >> > I've sent already.
>> >> Then it is going to break blocklayout user space program blkmapd, which is
>> >> stared before mounting any file system and it tries to open the pipe file
>> >> when started.
>> >
>> > Why on earth is blkmapd doing this instead of listening for file creation notifications like the other rpc_pipefs daemons do?
>> Not sure how the original implementer chose this but I think it is
>> likely because we do not expect the pipe file to be created or deleted
>> dynamically.
>
> Unless blkmapd can pin the sunrpc module (which it shouldn't be able to)
> then that assumption would be wrong. Please look into fixing blkmapd...
Sorry, I don't quite get it. Do you mean sunrpc module may be removed
while nfs/blocklayout modules are still in use? Please explain it a
bit. Thanks.

Best,
Tao
>
>   Trond
> --
> Trond Myklebust
> Linux NFS client maintainer
>
> NetApp
> [email protected]
> http://www.netapp.com
>



--
Thanks,
Tao

2011-11-29 13:16:33

by Stanislav Kinsbursky

[permalink] [raw]
Subject: Re: [PATCH 4/5] NFS: remove RPC PipeFS mount point reference from blocklayout routines

29.11.2011 16:40, [email protected] пишет:
>> -----Original Message-----
>> From: Stanislav Kinsbursky [mailto:[email protected]]
>> Sent: Tuesday, November 29, 2011 8:19 PM
>> To: Peng, Tao
>> Cc: [email protected]; [email protected]; Pavel Emelianov; [email protected];
>> [email protected]; [email protected]; James Bottomley; [email protected];
>> [email protected]; [email protected]
>> Subject: Re: [PATCH 4/5] NFS: remove RPC PipeFS mount point reference from blocklayout routines
>>
>> 29.11.2011 16:00, [email protected] пишет:
>>>> -----Original Message-----
>>>> From: [email protected] [mailto:[email protected]] On Behalf Of
>> Stanislav
>>>> Kinsbursky
>>>> Sent: Tuesday, November 29, 2011 6:11 PM
>>>> To: [email protected]
>>>> Cc: [email protected]; [email protected]; [email protected]; [email protected]; linux-
>>>> [email protected]; [email protected]; [email protected]; [email protected];
>>>> [email protected]
>>>> Subject: [PATCH 4/5] NFS: remove RPC PipeFS mount point reference from blocklayout routines
>>>>
>>>> This is a cleanup patch. We don't need this reference anymore, because
>>>> blocklayout pipes dentries now creates and destroys in per-net operations and
>>>> on PipeFS mount/umount notification.
>>>> Note that nfs4blocklayout_register_net() now returns 0 instead of -ENOENT in
>>>> case of PipeFS superblock absence. This is ok, because blocklayout pipe dentry
>>>> will be created on PipeFS mount event.
>>> When is the "pipefs mount event" going to happen? When inserting kernel modules or when user issues
>> mount command?
>>>
>>
>> When user issues mount command.
>> Kernel mounts of PipeFS has been removed with all these patch sets I've sent
>> already.
> Then it is going to break blocklayout user space program blkmapd, which is stared before mounting any file system and it tries to open the pipe file when started.

Sorry, but I don't get it. Probably we have misunderstanding here.
You said, that "blkmapd ... tries to open the pipe file when started". This pipe
file is located on PipeFS, isn't it?
If yes, then PipeFS have to be mounted already in user-space. And if it has been
mounted - then pipe dentry is present.
IOW, pipe (without dentry) will be created on module load. Pipe dentry will be
created right after that (like it was before) if PipeFS was mounted from
user-space. If not - then pipe dentry will be created on PipeFS (!) mount (not
NFS or pNFS mount) from user-space.

Or I'm missing something in your reply?

> Not sure if you implement the same logic on nfs pipe as well. But if you do, then nfs client user space program idmapd will fail to start for the same reason.
>

The same logic here.

> Why not just fail to load module if you fail to initialize pipefs? When is rpc_get_sb_net() going to fail?
>

Sorry, but I don't understand, what is your idea. And why do we need to fail at all.
BTW, rpc_get_sb_net() just checks, was PipeFS mounted in passed net, or not. If
not - not a problem. Dentries will be created on mount event. If yes, then it
returns locked PipeFS sb and the next step is dentry creation.

>>
>>
>>> Thanks,
>>> Tao
>>>
>>>>
>>>> Signed-off-by: Stanislav Kinsbursky<[email protected]>
>>>>
>>>> ---
>>>> fs/nfs/blocklayout/blocklayout.c | 9 +--------
>>>> 1 files changed, 1 insertions(+), 8 deletions(-)
>>>>
>>>> diff --git a/fs/nfs/blocklayout/blocklayout.c b/fs/nfs/blocklayout/blocklayout.c
>>>> index acf7ac9..8211ffd 100644
>>>> --- a/fs/nfs/blocklayout/blocklayout.c
>>>> +++ b/fs/nfs/blocklayout/blocklayout.c
>>>> @@ -1032,7 +1032,7 @@ static struct dentry *nfs4blocklayout_register_net(struct net *net,
>>>>
>>>> pipefs_sb = rpc_get_sb_net(net);
>>>> if (!pipefs_sb)
>>>> - return ERR_PTR(-ENOENT);
>>>> + return 0;
>>>> dentry = nfs4blocklayout_register_sb(pipefs_sb, pipe);
>>>> rpc_put_sb_net(net);
>>>> return dentry;
>>>> @@ -1083,7 +1083,6 @@ static struct pernet_operations nfs4blocklayout_net_ops = {
>>>>
>>>> static int __init nfs4blocklayout_init(void)
>>>> {
>>>> - struct vfsmount *mnt;
>>>> int ret;
>>>>
>>>> dprintk("%s: NFSv4 Block Layout Driver Registering...\n", __func__);
>>>> @@ -1093,12 +1092,6 @@ static int __init nfs4blocklayout_init(void)
>>>> goto out;
>>>>
>>>> init_waitqueue_head(&bl_wq);
>>>> -
>>>> - mnt = rpc_get_mount();
>>>> - if (IS_ERR(mnt)) {
>>>> - ret = PTR_ERR(mnt);
>>>> - goto out_remove;
>>>> - }
>>>> ret = rpc_pipefs_notifier_register(&nfs4blocklayout_block);
>>>> if (ret)
>>>> goto out_remove;
>>>>
>>>> --
>>>> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
>>>> the body of a message to [email protected]
>>>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>>
>>
>>
>> --
>> Best regards,
>> Stanislav Kinsbursky
>


--
Best regards,
Stanislav Kinsbursky

2011-11-29 12:41:22

by Peng, Tao

[permalink] [raw]
Subject: RE: [PATCH 4/5] NFS: remove RPC PipeFS mount point reference from blocklayout routines

PiAtLS0tLU9yaWdpbmFsIE1lc3NhZ2UtLS0tLQ0KPiBGcm9tOiBTdGFuaXNsYXYgS2luc2J1cnNr
eSBbbWFpbHRvOnNraW5zYnVyc2t5QHBhcmFsbGVscy5jb21dDQo+IFNlbnQ6IFR1ZXNkYXksIE5v
dmVtYmVyIDI5LCAyMDExIDg6MTkgUE0NCj4gVG86IFBlbmcsIFRhbw0KPiBDYzogVHJvbmQuTXlr
bGVidXN0QG5ldGFwcC5jb207IGxpbnV4LW5mc0B2Z2VyLmtlcm5lbC5vcmc7IFBhdmVsIEVtZWxp
YW5vdjsgbmVpbGJAc3VzZS5kZTsNCj4gbmV0ZGV2QHZnZXIua2VybmVsLm9yZzsgbGludXgta2Vy
bmVsQHZnZXIua2VybmVsLm9yZzsgSmFtZXMgQm90dG9tbGV5OyBiZmllbGRzQGZpZWxkc2VzLm9y
ZzsNCj4gZGF2ZW1AZGF2ZW1sb2Z0Lm5ldDsgZGV2ZWxAb3BlbnZ6Lm9yZw0KPiBTdWJqZWN0OiBS
ZTogW1BBVENIIDQvNV0gTkZTOiByZW1vdmUgUlBDIFBpcGVGUyBtb3VudCBwb2ludCByZWZlcmVu
Y2UgZnJvbSBibG9ja2xheW91dCByb3V0aW5lcw0KPiANCj4gMjkuMTEuMjAxMSAxNjowMCwgdGFv
LnBlbmdAZW1jLmNvbSDQv9C40YjQtdGCOg0KPiA+PiAtLS0tLU9yaWdpbmFsIE1lc3NhZ2UtLS0t
LQ0KPiA+PiBGcm9tOiBsaW51eC1uZnMtb3duZXJAdmdlci5rZXJuZWwub3JnIFttYWlsdG86bGlu
dXgtbmZzLW93bmVyQHZnZXIua2VybmVsLm9yZ10gT24gQmVoYWxmIE9mDQo+IFN0YW5pc2xhdg0K
PiA+PiBLaW5zYnVyc2t5DQo+ID4+IFNlbnQ6IFR1ZXNkYXksIE5vdmVtYmVyIDI5LCAyMDExIDY6
MTEgUE0NCj4gPj4gVG86IFRyb25kLk15a2xlYnVzdEBuZXRhcHAuY29tDQo+ID4+IENjOiBsaW51
eC1uZnNAdmdlci5rZXJuZWwub3JnOyB4ZW11bEBwYXJhbGxlbHMuY29tOyBuZWlsYkBzdXNlLmRl
OyBuZXRkZXZAdmdlci5rZXJuZWwub3JnOyBsaW51eC0NCj4gPj4ga2VybmVsQHZnZXIua2VybmVs
Lm9yZzsgamJvdHRvbWxleUBwYXJhbGxlbHMuY29tOyBiZmllbGRzQGZpZWxkc2VzLm9yZzsgZGF2
ZW1AZGF2ZW1sb2Z0Lm5ldDsNCj4gPj4gZGV2ZWxAb3BlbnZ6Lm9yZw0KPiA+PiBTdWJqZWN0OiBb
UEFUQ0ggNC81XSBORlM6IHJlbW92ZSBSUEMgUGlwZUZTIG1vdW50IHBvaW50IHJlZmVyZW5jZSBm
cm9tIGJsb2NrbGF5b3V0IHJvdXRpbmVzDQo+ID4+DQo+ID4+IFRoaXMgaXMgYSBjbGVhbnVwIHBh
dGNoLiBXZSBkb24ndCBuZWVkIHRoaXMgcmVmZXJlbmNlIGFueW1vcmUsIGJlY2F1c2UNCj4gPj4g
YmxvY2tsYXlvdXQgcGlwZXMgZGVudHJpZXMgbm93IGNyZWF0ZXMgYW5kIGRlc3Ryb3lzIGluIHBl
ci1uZXQgb3BlcmF0aW9ucyBhbmQNCj4gPj4gb24gUGlwZUZTIG1vdW50L3Vtb3VudCBub3RpZmlj
YXRpb24uDQo+ID4+IE5vdGUgdGhhdCBuZnM0YmxvY2tsYXlvdXRfcmVnaXN0ZXJfbmV0KCkgbm93
IHJldHVybnMgMCBpbnN0ZWFkIG9mIC1FTk9FTlQgaW4NCj4gPj4gY2FzZSBvZiBQaXBlRlMgc3Vw
ZXJibG9jayBhYnNlbmNlLiBUaGlzIGlzIG9rLCBiZWNhdXNlIGJsb2NrbGF5b3V0IHBpcGUgZGVu
dHJ5DQo+ID4+IHdpbGwgYmUgY3JlYXRlZCBvbiBQaXBlRlMgbW91bnQgZXZlbnQuDQo+ID4gV2hl
biBpcyB0aGUgInBpcGVmcyBtb3VudCBldmVudCIgZ29pbmcgdG8gaGFwcGVuPyBXaGVuIGluc2Vy
dGluZyBrZXJuZWwgbW9kdWxlcyBvciB3aGVuIHVzZXIgaXNzdWVzDQo+IG1vdW50IGNvbW1hbmQ/
DQo+ID4NCj4gDQo+IFdoZW4gdXNlciBpc3N1ZXMgbW91bnQgY29tbWFuZC4NCj4gS2VybmVsIG1v
dW50cyBvZiBQaXBlRlMgaGFzIGJlZW4gcmVtb3ZlZCB3aXRoIGFsbCB0aGVzZSBwYXRjaCBzZXRz
IEkndmUgc2VudA0KPiBhbHJlYWR5Lg0KVGhlbiBpdCBpcyBnb2luZyB0byBicmVhayBibG9ja2xh
eW91dCB1c2VyIHNwYWNlIHByb2dyYW0gYmxrbWFwZCwgd2hpY2ggaXMgc3RhcmVkIGJlZm9yZSBt
b3VudGluZyBhbnkgZmlsZSBzeXN0ZW0gYW5kIGl0IHRyaWVzIHRvIG9wZW4gdGhlIHBpcGUgZmls
ZSB3aGVuIHN0YXJ0ZWQuDQpOb3Qgc3VyZSBpZiB5b3UgaW1wbGVtZW50IHRoZSBzYW1lIGxvZ2lj
IG9uIG5mcyBwaXBlIGFzIHdlbGwuIEJ1dCBpZiB5b3UgZG8sIHRoZW4gbmZzIGNsaWVudCB1c2Vy
IHNwYWNlIHByb2dyYW0gaWRtYXBkIHdpbGwgZmFpbCB0byBzdGFydCBmb3IgdGhlIHNhbWUgcmVh
c29uLg0KDQpXaHkgbm90IGp1c3QgZmFpbCB0byBsb2FkIG1vZHVsZSBpZiB5b3UgZmFpbCB0byBp
bml0aWFsaXplIHBpcGVmcz8gV2hlbiBpcyBycGNfZ2V0X3NiX25ldCgpIGdvaW5nIHRvIGZhaWw/
DQoNCj4gDQo+IA0KPiA+IFRoYW5rcywNCj4gPiBUYW8NCj4gPg0KPiA+Pg0KPiA+PiBTaWduZWQt
b2ZmLWJ5OiBTdGFuaXNsYXYgS2luc2J1cnNreTxza2luc2J1cnNreUBwYXJhbGxlbHMuY29tPg0K
PiA+Pg0KPiA+PiAtLS0NCj4gPj4gICBmcy9uZnMvYmxvY2tsYXlvdXQvYmxvY2tsYXlvdXQuYyB8
ICAgIDkgKy0tLS0tLS0tDQo+ID4+ICAgMSBmaWxlcyBjaGFuZ2VkLCAxIGluc2VydGlvbnMoKyks
IDggZGVsZXRpb25zKC0pDQo+ID4+DQo+ID4+IGRpZmYgLS1naXQgYS9mcy9uZnMvYmxvY2tsYXlv
dXQvYmxvY2tsYXlvdXQuYyBiL2ZzL25mcy9ibG9ja2xheW91dC9ibG9ja2xheW91dC5jDQo+ID4+
IGluZGV4IGFjZjdhYzkuLjgyMTFmZmQgMTAwNjQ0DQo+ID4+IC0tLSBhL2ZzL25mcy9ibG9ja2xh
eW91dC9ibG9ja2xheW91dC5jDQo+ID4+ICsrKyBiL2ZzL25mcy9ibG9ja2xheW91dC9ibG9ja2xh
eW91dC5jDQo+ID4+IEBAIC0xMDMyLDcgKzEwMzIsNyBAQCBzdGF0aWMgc3RydWN0IGRlbnRyeSAq
bmZzNGJsb2NrbGF5b3V0X3JlZ2lzdGVyX25ldChzdHJ1Y3QgbmV0ICpuZXQsDQo+ID4+DQo+ID4+
ICAgCXBpcGVmc19zYiA9IHJwY19nZXRfc2JfbmV0KG5ldCk7DQo+ID4+ICAgCWlmICghcGlwZWZz
X3NiKQ0KPiA+PiAtCQlyZXR1cm4gRVJSX1BUUigtRU5PRU5UKTsNCj4gPj4gKwkJcmV0dXJuIDA7
DQo+ID4+ICAgCWRlbnRyeSA9IG5mczRibG9ja2xheW91dF9yZWdpc3Rlcl9zYihwaXBlZnNfc2Is
IHBpcGUpOw0KPiA+PiAgIAlycGNfcHV0X3NiX25ldChuZXQpOw0KPiA+PiAgIAlyZXR1cm4gZGVu
dHJ5Ow0KPiA+PiBAQCAtMTA4Myw3ICsxMDgzLDYgQEAgc3RhdGljIHN0cnVjdCBwZXJuZXRfb3Bl
cmF0aW9ucyBuZnM0YmxvY2tsYXlvdXRfbmV0X29wcyA9IHsNCj4gPj4NCj4gPj4gICBzdGF0aWMg
aW50IF9faW5pdCBuZnM0YmxvY2tsYXlvdXRfaW5pdCh2b2lkKQ0KPiA+PiAgIHsNCj4gPj4gLQlz
dHJ1Y3QgdmZzbW91bnQgKm1udDsNCj4gPj4gICAJaW50IHJldDsNCj4gPj4NCj4gPj4gICAJZHBy
aW50aygiJXM6IE5GU3Y0IEJsb2NrIExheW91dCBEcml2ZXIgUmVnaXN0ZXJpbmcuLi5cbiIsIF9f
ZnVuY19fKTsNCj4gPj4gQEAgLTEwOTMsMTIgKzEwOTIsNiBAQCBzdGF0aWMgaW50IF9faW5pdCBu
ZnM0YmxvY2tsYXlvdXRfaW5pdCh2b2lkKQ0KPiA+PiAgIAkJZ290byBvdXQ7DQo+ID4+DQo+ID4+
ICAgCWluaXRfd2FpdHF1ZXVlX2hlYWQoJmJsX3dxKTsNCj4gPj4gLQ0KPiA+PiAtCW1udCA9IHJw
Y19nZXRfbW91bnQoKTsNCj4gPj4gLQlpZiAoSVNfRVJSKG1udCkpIHsNCj4gPj4gLQkJcmV0ID0g
UFRSX0VSUihtbnQpOw0KPiA+PiAtCQlnb3RvIG91dF9yZW1vdmU7DQo+ID4+IC0JfQ0KPiA+PiAg
IAlyZXQgPSBycGNfcGlwZWZzX25vdGlmaWVyX3JlZ2lzdGVyKCZuZnM0YmxvY2tsYXlvdXRfYmxv
Y2spOw0KPiA+PiAgIAlpZiAocmV0KQ0KPiA+PiAgIAkJZ290byBvdXRfcmVtb3ZlOw0KPiA+Pg0K
PiA+PiAtLQ0KPiA+PiBUbyB1bnN1YnNjcmliZSBmcm9tIHRoaXMgbGlzdDogc2VuZCB0aGUgbGlu
ZSAidW5zdWJzY3JpYmUgbGludXgtbmZzIiBpbg0KPiA+PiB0aGUgYm9keSBvZiBhIG1lc3NhZ2Ug
dG8gbWFqb3Jkb21vQHZnZXIua2VybmVsLm9yZw0KPiA+PiBNb3JlIG1ham9yZG9tbyBpbmZvIGF0
ICBodHRwOi8vdmdlci5rZXJuZWwub3JnL21ham9yZG9tby1pbmZvLmh0bWwNCj4gPg0KPiANCj4g
DQo+IC0tDQo+IEJlc3QgcmVnYXJkcywNCj4gU3RhbmlzbGF2IEtpbnNidXJza3kNCg0K

2011-11-29 17:30:49

by Peng Tao

[permalink] [raw]
Subject: Re: [PATCH 4/5] NFS: remove RPC PipeFS mount point reference from blocklayout routines

On Wed, Nov 30, 2011 at 1:19 AM, Trond Myklebust
<[email protected]> wrote:
> On Tue, 2011-11-29 at 11:42 -0500, J. Bruce Fields wrote:
>> On Tue, Nov 29, 2011 at 11:40:30AM -0500, Trond Myklebust wrote:
>> > I mean that I'm perfectly entitled to do
>> >
>> > 'modprobe -r blocklayoutdriver'
>> >
>> > and when I do that, then I expect blkmapd to close the rpc pipe and wait
>> > for a new one to be created just like rpc.idmapd and rpc.gssd do when I
>> > remove the nfs and sunrpc modules.
>>
>> The rpc pipefs mount doesn't hold a reference on the sunrpc module?
>
> I stand corrected: the mount does hold a reference to the sunrpc
> module.
> However nothing holds a reference to the blocklayoutdriver module, so
> the main point that the "blocklayout" pipe can disappear from underneath
> the blkmapd stands.
Thanks for the explanation and I agree it can cause problem if user
reload blocklayout module. I will look into a fix to blkmapd.

Best,
Tao

2011-11-29 16:42:58

by J. Bruce Fields

[permalink] [raw]
Subject: Re: [PATCH 4/5] NFS: remove RPC PipeFS mount point reference from blocklayout routines

On Tue, Nov 29, 2011 at 11:40:30AM -0500, Trond Myklebust wrote:
> I mean that I'm perfectly entitled to do
>
> 'modprobe -r blocklayoutdriver'
>
> and when I do that, then I expect blkmapd to close the rpc pipe and wait
> for a new one to be created just like rpc.idmapd and rpc.gssd do when I
> remove the nfs and sunrpc modules.

The rpc pipefs mount doesn't hold a reference on the sunrpc module?

--b.

2011-11-29 13:36:15

by Myklebust, Trond

[permalink] [raw]
Subject: RE: [PATCH 4/5] NFS: remove RPC PipeFS mount point reference from blocklayout routines

PiAtLS0tLU9yaWdpbmFsIE1lc3NhZ2UtLS0tLQ0KPiBGcm9tOiB0YW8ucGVuZ0BlbWMuY29tIFtt
YWlsdG86dGFvLnBlbmdAZW1jLmNvbV0NCj4gU2VudDogVHVlc2RheSwgTm92ZW1iZXIgMjksIDIw
MTEgNzo0MCBBTQ0KPiBUbzogc2tpbnNidXJza3lAcGFyYWxsZWxzLmNvbQ0KPiBDYzogTXlrbGVi
dXN0LCBUcm9uZDsgbGludXgtbmZzQHZnZXIua2VybmVsLm9yZzsgeGVtdWxAcGFyYWxsZWxzLmNv
bTsNCj4gbmVpbGJAc3VzZS5kZTsgbmV0ZGV2QHZnZXIua2VybmVsLm9yZzsgbGludXgta2VybmVs
QHZnZXIua2VybmVsLm9yZzsNCj4gamJvdHRvbWxleUBwYXJhbGxlbHMuY29tOyBiZmllbGRzQGZp
ZWxkc2VzLm9yZzsgZGF2ZW1AZGF2ZW1sb2Z0Lm5ldDsNCj4gZGV2ZWxAb3BlbnZ6Lm9yZw0KPiBT
dWJqZWN0OiBSRTogW1BBVENIIDQvNV0gTkZTOiByZW1vdmUgUlBDIFBpcGVGUyBtb3VudCBwb2lu
dCByZWZlcmVuY2UNCj4gZnJvbSBibG9ja2xheW91dCByb3V0aW5lcw0KPiANCj4gPiAtLS0tLU9y
aWdpbmFsIE1lc3NhZ2UtLS0tLQ0KPiA+IEZyb206IFN0YW5pc2xhdiBLaW5zYnVyc2t5IFttYWls
dG86c2tpbnNidXJza3lAcGFyYWxsZWxzLmNvbV0NCj4gPiBTZW50OiBUdWVzZGF5LCBOb3ZlbWJl
ciAyOSwgMjAxMSA4OjE5IFBNDQo+ID4gVG86IFBlbmcsIFRhbw0KPiA+IENjOiBUcm9uZC5NeWts
ZWJ1c3RAbmV0YXBwLmNvbTsgbGludXgtbmZzQHZnZXIua2VybmVsLm9yZzsgUGF2ZWwNCj4gPiBF
bWVsaWFub3Y7IG5laWxiQHN1c2UuZGU7IG5ldGRldkB2Z2VyLmtlcm5lbC5vcmc7DQo+ID4gbGlu
dXgta2VybmVsQHZnZXIua2VybmVsLm9yZzsgSmFtZXMgQm90dG9tbGV5OyBiZmllbGRzQGZpZWxk
c2VzLm9yZzsNCj4gPiBkYXZlbUBkYXZlbWxvZnQubmV0OyBkZXZlbEBvcGVudnoub3JnDQo+ID4g
U3ViamVjdDogUmU6IFtQQVRDSCA0LzVdIE5GUzogcmVtb3ZlIFJQQyBQaXBlRlMgbW91bnQgcG9p
bnQgcmVmZXJlbmNlDQo+ID4gZnJvbSBibG9ja2xheW91dCByb3V0aW5lcw0KPiA+DQo+ID4gMjku
MTEuMjAxMSAxNjowMCwgdGFvLnBlbmdAZW1jLmNvbSDQv9C40YjQtdGCOg0KPiA+ID4+IC0tLS0t
T3JpZ2luYWwgTWVzc2FnZS0tLS0tDQo+ID4gPj4gRnJvbTogbGludXgtbmZzLW93bmVyQHZnZXIu
a2VybmVsLm9yZw0KPiA+ID4+IFttYWlsdG86bGludXgtbmZzLW93bmVyQHZnZXIua2VybmVsLm9y
Z10gT24gQmVoYWxmIE9mDQo+ID4gU3RhbmlzbGF2DQo+ID4gPj4gS2luc2J1cnNreQ0KPiA+ID4+
IFNlbnQ6IFR1ZXNkYXksIE5vdmVtYmVyIDI5LCAyMDExIDY6MTEgUE0NCj4gPiA+PiBUbzogVHJv
bmQuTXlrbGVidXN0QG5ldGFwcC5jb20NCj4gPiA+PiBDYzogbGludXgtbmZzQHZnZXIua2VybmVs
Lm9yZzsgeGVtdWxAcGFyYWxsZWxzLmNvbTsgbmVpbGJAc3VzZS5kZTsNCj4gPiA+PiBuZXRkZXZA
dmdlci5rZXJuZWwub3JnOyBsaW51eC0ga2VybmVsQHZnZXIua2VybmVsLm9yZzsNCj4gPiA+PiBq
Ym90dG9tbGV5QHBhcmFsbGVscy5jb207IGJmaWVsZHNAZmllbGRzZXMub3JnOw0KPiA+ID4+IGRh
dmVtQGRhdmVtbG9mdC5uZXQ7IGRldmVsQG9wZW52ei5vcmcNCj4gPiA+PiBTdWJqZWN0OiBbUEFU
Q0ggNC81XSBORlM6IHJlbW92ZSBSUEMgUGlwZUZTIG1vdW50IHBvaW50IHJlZmVyZW5jZQ0KPiA+
ID4+IGZyb20gYmxvY2tsYXlvdXQgcm91dGluZXMNCj4gPiA+Pg0KPiA+ID4+IFRoaXMgaXMgYSBj
bGVhbnVwIHBhdGNoLiBXZSBkb24ndCBuZWVkIHRoaXMgcmVmZXJlbmNlIGFueW1vcmUsDQo+ID4g
Pj4gYmVjYXVzZSBibG9ja2xheW91dCBwaXBlcyBkZW50cmllcyBub3cgY3JlYXRlcyBhbmQgZGVz
dHJveXMgaW4NCj4gPiA+PiBwZXItbmV0IG9wZXJhdGlvbnMgYW5kIG9uIFBpcGVGUyBtb3VudC91
bW91bnQgbm90aWZpY2F0aW9uLg0KPiA+ID4+IE5vdGUgdGhhdCBuZnM0YmxvY2tsYXlvdXRfcmVn
aXN0ZXJfbmV0KCkgbm93IHJldHVybnMgMCBpbnN0ZWFkIG9mDQo+ID4gPj4gLUVOT0VOVCBpbiBj
YXNlIG9mIFBpcGVGUyBzdXBlcmJsb2NrIGFic2VuY2UuIFRoaXMgaXMgb2ssIGJlY2F1c2UNCj4g
PiA+PiBibG9ja2xheW91dCBwaXBlIGRlbnRyeSB3aWxsIGJlIGNyZWF0ZWQgb24gUGlwZUZTIG1v
dW50IGV2ZW50Lg0KPiA+ID4gV2hlbiBpcyB0aGUgInBpcGVmcyBtb3VudCBldmVudCIgZ29pbmcg
dG8gaGFwcGVuPyBXaGVuIGluc2VydGluZw0KPiA+ID4ga2VybmVsIG1vZHVsZXMgb3Igd2hlbiB1
c2VyIGlzc3Vlcw0KPiA+IG1vdW50IGNvbW1hbmQ/DQo+ID4gPg0KPiA+DQo+ID4gV2hlbiB1c2Vy
IGlzc3VlcyBtb3VudCBjb21tYW5kLg0KPiA+IEtlcm5lbCBtb3VudHMgb2YgUGlwZUZTIGhhcyBi
ZWVuIHJlbW92ZWQgd2l0aCBhbGwgdGhlc2UgcGF0Y2ggc2V0cw0KPiA+IEkndmUgc2VudCBhbHJl
YWR5Lg0KPiBUaGVuIGl0IGlzIGdvaW5nIHRvIGJyZWFrIGJsb2NrbGF5b3V0IHVzZXIgc3BhY2Ug
cHJvZ3JhbSBibGttYXBkLCB3aGljaCBpcw0KPiBzdGFyZWQgYmVmb3JlIG1vdW50aW5nIGFueSBm
aWxlIHN5c3RlbSBhbmQgaXQgdHJpZXMgdG8gb3BlbiB0aGUgcGlwZSBmaWxlDQo+IHdoZW4gc3Rh
cnRlZC4NCg0KV2h5IG9uIGVhcnRoIGlzIGJsa21hcGQgZG9pbmcgdGhpcyBpbnN0ZWFkIG9mIGxp
c3RlbmluZyBmb3IgZmlsZSBjcmVhdGlvbiBub3RpZmljYXRpb25zIGxpa2UgdGhlIG90aGVyIHJw
Y19waXBlZnMgZGFlbW9ucyBkbz8NCg0KVHJvbmQNCg0K

2011-11-29 15:18:44

by Myklebust, Trond

[permalink] [raw]
Subject: Re: [PATCH 4/5] NFS: remove RPC PipeFS mount point reference from blocklayout routines

On Tue, 2011-11-29 at 23:10 +0800, Peng Tao wrote:
> On Tue, Nov 29, 2011 at 9:35 PM, Myklebust, Trond
> <[email protected]> wrote:
> >> -----Original Message-----
> >> From: [email protected] [mailto:[email protected]]
> >> Sent: Tuesday, November 29, 2011 7:40 AM
> >> To: [email protected]
> >> Cc: Myklebust, Trond; [email protected]; [email protected];
> >> [email protected]; [email protected]; [email protected];
> >> [email protected]; [email protected]; [email protected];
> >> [email protected]
> >> Subject: RE: [PATCH 4/5] NFS: remove RPC PipeFS mount point reference
> >> from blocklayout routines
> >>
> >> > -----Original Message-----
> >> > From: Stanislav Kinsbursky [mailto:[email protected]]
> >> > Sent: Tuesday, November 29, 2011 8:19 PM
> >> > To: Peng, Tao
> >> > Cc: [email protected]; [email protected]; Pavel
> >> > Emelianov; [email protected]; [email protected];
> >> > [email protected]; James Bottomley; [email protected];
> >> > [email protected]; [email protected]
> >> > Subject: Re: [PATCH 4/5] NFS: remove RPC PipeFS mount point reference
> >> > from blocklayout routines
> >> >
> >> > 29.11.2011 16:00, [email protected] пишет:
> >> > >> -----Original Message-----
> >> > >> From: [email protected]
> >> > >> [mailto:[email protected]] On Behalf Of
> >> > Stanislav
> >> > >> Kinsbursky
> >> > >> Sent: Tuesday, November 29, 2011 6:11 PM
> >> > >> To: [email protected]
> >> > >> Cc: [email protected]; [email protected]; [email protected];
> >> > >> [email protected]; linux- [email protected];
> >> > >> [email protected]; [email protected];
> >> > >> [email protected]; [email protected]
> >> > >> Subject: [PATCH 4/5] NFS: remove RPC PipeFS mount point reference
> >> > >> from blocklayout routines
> >> > >>
> >> > >> This is a cleanup patch. We don't need this reference anymore,
> >> > >> because blocklayout pipes dentries now creates and destroys in
> >> > >> per-net operations and on PipeFS mount/umount notification.
> >> > >> Note that nfs4blocklayout_register_net() now returns 0 instead of
> >> > >> -ENOENT in case of PipeFS superblock absence. This is ok, because
> >> > >> blocklayout pipe dentry will be created on PipeFS mount event.
> >> > > When is the "pipefs mount event" going to happen? When inserting
> >> > > kernel modules or when user issues
> >> > mount command?
> >> > >
> >> >
> >> > When user issues mount command.
> >> > Kernel mounts of PipeFS has been removed with all these patch sets
> >> > I've sent already.
> >> Then it is going to break blocklayout user space program blkmapd, which is
> >> stared before mounting any file system and it tries to open the pipe file
> >> when started.
> >
> > Why on earth is blkmapd doing this instead of listening for file creation notifications like the other rpc_pipefs daemons do?
> Not sure how the original implementer chose this but I think it is
> likely because we do not expect the pipe file to be created or deleted
> dynamically.

Unless blkmapd can pin the sunrpc module (which it shouldn't be able to)
then that assumption would be wrong. Please look into fixing blkmapd...

Trond
--
Trond Myklebust
Linux NFS client maintainer

NetApp
[email protected]
http://www.netapp.com


2011-11-29 15:05:55

by Peng Tao

[permalink] [raw]
Subject: Re: [PATCH 4/5] NFS: remove RPC PipeFS mount point reference from blocklayout routines

On Tue, Nov 29, 2011 at 9:13 PM, Stanislav Kinsbursky
<[email protected]> wrote:
> 29.11.2011 16:40, [email protected] пишет:
>
>>> -----Original Message-----
>>> From: Stanislav Kinsbursky [mailto:[email protected]]
>>> Sent: Tuesday, November 29, 2011 8:19 PM
>>> To: Peng, Tao
>>> Cc: [email protected]; [email protected]; Pavel
>>> Emelianov; [email protected];
>>> [email protected]; [email protected]; James Bottomley;
>>> [email protected];
>>> [email protected]; [email protected]
>>> Subject: Re: [PATCH 4/5] NFS: remove RPC PipeFS mount point reference
>>> from blocklayout routines
>>>
>>> 29.11.2011 16:00, [email protected] пишет:
>>>>>
>>>>> -----Original Message-----
>>>>> From: [email protected]
>>>>> [mailto:[email protected]] On Behalf Of
>>>
>>> Stanislav
>>>>>
>>>>> Kinsbursky
>>>>> Sent: Tuesday, November 29, 2011 6:11 PM
>>>>> To: [email protected]
>>>>> Cc: [email protected]; [email protected]; [email protected];
>>>>> [email protected]; linux-
>>>>> [email protected]; [email protected]; [email protected];
>>>>> [email protected];
>>>>> [email protected]
>>>>> Subject: [PATCH 4/5] NFS: remove RPC PipeFS mount point reference from
>>>>> blocklayout routines
>>>>>
>>>>> This is a cleanup patch. We don't need this reference anymore, because
>>>>> blocklayout pipes dentries now creates and destroys in per-net
>>>>> operations and
>>>>> on PipeFS mount/umount notification.
>>>>> Note that nfs4blocklayout_register_net() now returns 0 instead of
>>>>> -ENOENT in
>>>>> case of PipeFS superblock absence. This is ok, because blocklayout pipe
>>>>> dentry
>>>>> will be created on PipeFS mount event.
>>>>
>>>> When is the "pipefs mount event" going to happen? When inserting kernel
>>>> modules or when user issues
>>>
>>> mount command?
>>>>
>>>>
>>>
>>> When user issues mount command.
>>> Kernel mounts of PipeFS has been removed with all these patch sets I've
>>> sent
>>> already.
>>
>> Then it is going to break blocklayout user space program blkmapd, which is
>> stared before mounting any file system and it tries to open the pipe file
>> when started.
>
>
> Sorry, but I don't get it. Probably we have misunderstanding here.
> You said, that "blkmapd ... tries to open the pipe file when started". This
> pipe file is located on PipeFS, isn't it?
> If yes, then PipeFS have to be mounted already in user-space. And if it has
> been mounted - then pipe dentry is present.
> IOW, pipe (without dentry) will be created on module load. Pipe dentry will
> be created right after that (like it was before) if PipeFS was mounted from
> user-space. If not - then pipe dentry will be created  on PipeFS (!) mount
> (not NFS or pNFS mount) from user-space.
Sorry, I misunderstood. I was thinking about mounting NFS or pNFS when
you say "when user issues mount command". Thanks for the explanation.

Regards,
Tao
>
> Or I'm missing something in your reply?
>
>
>> Not sure if you implement the same logic on nfs pipe as well. But if you
>> do, then nfs client user space program idmapd will fail to start for the
>> same reason.
>>
>
> The same logic here.
>
>
>> Why not just fail to load module if you fail to initialize pipefs? When is
>> rpc_get_sb_net() going to fail?
>>
>
> Sorry, but I don't understand, what is your idea. And why do we need to fail
> at all.
> BTW, rpc_get_sb_net() just checks, was PipeFS mounted in passed net, or not.
> If not - not a problem. Dentries will be created on mount event. If yes,
> then it returns locked PipeFS sb and the next step is dentry creation.
>
>
>>>
>>>
>>>> Thanks,
>>>> Tao
>>>>
>>>>>
>>>>> Signed-off-by: Stanislav Kinsbursky<[email protected]>
>>>>>
>>>>> ---
>>>>>   fs/nfs/blocklayout/blocklayout.c |    9 +--------
>>>>>   1 files changed, 1 insertions(+), 8 deletions(-)
>>>>>
>>>>> diff --git a/fs/nfs/blocklayout/blocklayout.c
>>>>> b/fs/nfs/blocklayout/blocklayout.c
>>>>> index acf7ac9..8211ffd 100644
>>>>> --- a/fs/nfs/blocklayout/blocklayout.c
>>>>> +++ b/fs/nfs/blocklayout/blocklayout.c
>>>>> @@ -1032,7 +1032,7 @@ static struct dentry
>>>>> *nfs4blocklayout_register_net(struct net *net,
>>>>>
>>>>>        pipefs_sb = rpc_get_sb_net(net);
>>>>>        if (!pipefs_sb)
>>>>> -               return ERR_PTR(-ENOENT);
>>>>> +               return 0;
>>>>>        dentry = nfs4blocklayout_register_sb(pipefs_sb, pipe);
>>>>>        rpc_put_sb_net(net);
>>>>>        return dentry;
>>>>> @@ -1083,7 +1083,6 @@ static struct pernet_operations
>>>>> nfs4blocklayout_net_ops = {
>>>>>
>>>>>   static int __init nfs4blocklayout_init(void)
>>>>>   {
>>>>> -       struct vfsmount *mnt;
>>>>>        int ret;
>>>>>
>>>>>        dprintk("%s: NFSv4 Block Layout Driver Registering...\n",
>>>>> __func__);
>>>>> @@ -1093,12 +1092,6 @@ static int __init nfs4blocklayout_init(void)
>>>>>                goto out;
>>>>>
>>>>>        init_waitqueue_head(&bl_wq);
>>>>> -
>>>>> -       mnt = rpc_get_mount();
>>>>> -       if (IS_ERR(mnt)) {
>>>>> -               ret = PTR_ERR(mnt);
>>>>> -               goto out_remove;
>>>>> -       }
>>>>>        ret = rpc_pipefs_notifier_register(&nfs4blocklayout_block);
>>>>>        if (ret)
>>>>>                goto out_remove;
>>>>>
>>>>> --
>>>>> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
>>>>> the body of a message to [email protected]
>>>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>>>
>>>>
>>>
>>>
>>> --
>>> Best regards,
>>> Stanislav Kinsbursky
>>
>>
>
>
> --
> Best regards,
> Stanislav Kinsbursky
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to [email protected]
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

2011-11-29 09:12:17

by Stanislav Kinsbursky

[permalink] [raw]
Subject: [PATCH 1/5] NFS: handle blocklayout pipe PipeFS dentry by network namespace aware routines

This patch makes blocklayout pipe dentry allocated and destroyed in network
namespace context by PipeFS network namespace aware routines.
Network namespace context is obtained from nfs_client structure.

Signed-off-by: Stanislav Kinsbursky <[email protected]>

---
fs/nfs/blocklayout/blocklayout.c | 60 +++++++++++++++++++++++++++++++-------
1 files changed, 49 insertions(+), 11 deletions(-)

diff --git a/fs/nfs/blocklayout/blocklayout.c b/fs/nfs/blocklayout/blocklayout.c
index c26633e..50d5183 100644
--- a/fs/nfs/blocklayout/blocklayout.c
+++ b/fs/nfs/blocklayout/blocklayout.c
@@ -965,10 +965,55 @@ static const struct rpc_pipe_ops bl_upcall_ops = {
.destroy_msg = bl_pipe_destroy_msg,
};

+static struct dentry *nfs4blocklayout_register_sb(struct super_block *sb,
+ struct rpc_pipe *pipe)
+{
+ struct dentry *dir, *dentry;
+
+ dir = rpc_d_lookup_sb(sb, NFS_PIPE_DIRNAME);
+ if (dir == NULL)
+ return ERR_PTR(-ENOENT);
+ dentry = rpc_mkpipe_dentry(dir, "blocklayout", NULL, pipe);
+ dput(dir);
+ return dentry;
+}
+
+static void nfs4blocklayout_unregister_sb(struct super_block *sb,
+ struct rpc_pipe *pipe)
+{
+ if (pipe->dentry)
+ rpc_unlink(pipe->dentry);
+}
+
+static struct dentry *nfs4blocklayout_register_net(struct net *net,
+ struct rpc_pipe *pipe)
+{
+ struct super_block *pipefs_sb;
+ struct dentry *dentry;
+
+ pipefs_sb = rpc_get_sb_net(net);
+ if (!pipefs_sb)
+ return ERR_PTR(-ENOENT);
+ dentry = nfs4blocklayout_register_sb(pipefs_sb, pipe);
+ rpc_put_sb_net(net);
+ return dentry;
+}
+
+static void nfs4blocklayout_unregister_net(struct net *net,
+ struct rpc_pipe *pipe)
+{
+ struct super_block *pipefs_sb;
+
+ pipefs_sb = rpc_get_sb_net(net);
+ if (pipefs_sb) {
+ nfs4blocklayout_unregister_sb(pipefs_sb, pipe);
+ rpc_put_sb_net(net);
+ }
+}
+
static int __init nfs4blocklayout_init(void)
{
struct vfsmount *mnt;
- struct path path;
int ret;

dprintk("%s: NFSv4 Block Layout Driver Registering...\n", __func__);
@@ -984,20 +1029,13 @@ static int __init nfs4blocklayout_init(void)
ret = PTR_ERR(mnt);
goto out_remove;
}
-
- ret = vfs_path_lookup(mnt->mnt_root,
- mnt,
- NFS_PIPE_DIRNAME, 0, &path);
- if (ret)
- goto out_remove;
-
bl_device_pipe = rpc_mkpipe_data(&bl_upcall_ops, 0);
if (IS_ERR(bl_device_pipe)) {
ret = PTR_ERR(bl_device_pipe);
goto out_remove;
}
- bl_device_pipe->dentry = rpc_mkpipe_dentry(path.dentry, "blocklayout",
- NULL, bl_device_pipe);
+ bl_device_pipe->dentry = nfs4blocklayout_register_net(&init_net,
+ bl_device_pipe);
if (IS_ERR(bl_device_pipe->dentry)) {
ret = PTR_ERR(bl_device_pipe->dentry);
goto out_destroy_pipe;
@@ -1018,7 +1056,7 @@ static void __exit nfs4blocklayout_exit(void)
__func__);

pnfs_unregister_layoutdriver(&blocklayout_type);
- rpc_unlink(bl_device_pipe->dentry);
+ nfs4blocklayout_unregister_net(&init_net, bl_device_pipe);
rpc_destroy_pipe_data(bl_device_pipe);
}



2011-11-29 12:21:44

by Stanislav Kinsbursky

[permalink] [raw]
Subject: Re: [PATCH 4/5] NFS: remove RPC PipeFS mount point reference from blocklayout routines

29.11.2011 16:00, [email protected] пишет:
>> -----Original Message-----
>> From: [email protected] [mailto:[email protected]] On Behalf Of Stanislav
>> Kinsbursky
>> Sent: Tuesday, November 29, 2011 6:11 PM
>> To: [email protected]
>> Cc: [email protected]; [email protected]; [email protected]; [email protected]; linux-
>> [email protected]; [email protected]; [email protected]; [email protected];
>> [email protected]
>> Subject: [PATCH 4/5] NFS: remove RPC PipeFS mount point reference from blocklayout routines
>>
>> This is a cleanup patch. We don't need this reference anymore, because
>> blocklayout pipes dentries now creates and destroys in per-net operations and
>> on PipeFS mount/umount notification.
>> Note that nfs4blocklayout_register_net() now returns 0 instead of -ENOENT in
>> case of PipeFS superblock absence. This is ok, because blocklayout pipe dentry
>> will be created on PipeFS mount event.
> When is the "pipefs mount event" going to happen? When inserting kernel modules or when user issues mount command?
>

When user issues mount command.
Kernel mounts of PipeFS has been removed with all these patch sets I've sent
already.


> Thanks,
> Tao
>
>>
>> Signed-off-by: Stanislav Kinsbursky<[email protected]>
>>
>> ---
>> fs/nfs/blocklayout/blocklayout.c | 9 +--------
>> 1 files changed, 1 insertions(+), 8 deletions(-)
>>
>> diff --git a/fs/nfs/blocklayout/blocklayout.c b/fs/nfs/blocklayout/blocklayout.c
>> index acf7ac9..8211ffd 100644
>> --- a/fs/nfs/blocklayout/blocklayout.c
>> +++ b/fs/nfs/blocklayout/blocklayout.c
>> @@ -1032,7 +1032,7 @@ static struct dentry *nfs4blocklayout_register_net(struct net *net,
>>
>> pipefs_sb = rpc_get_sb_net(net);
>> if (!pipefs_sb)
>> - return ERR_PTR(-ENOENT);
>> + return 0;
>> dentry = nfs4blocklayout_register_sb(pipefs_sb, pipe);
>> rpc_put_sb_net(net);
>> return dentry;
>> @@ -1083,7 +1083,6 @@ static struct pernet_operations nfs4blocklayout_net_ops = {
>>
>> static int __init nfs4blocklayout_init(void)
>> {
>> - struct vfsmount *mnt;
>> int ret;
>>
>> dprintk("%s: NFSv4 Block Layout Driver Registering...\n", __func__);
>> @@ -1093,12 +1092,6 @@ static int __init nfs4blocklayout_init(void)
>> goto out;
>>
>> init_waitqueue_head(&bl_wq);
>> -
>> - mnt = rpc_get_mount();
>> - if (IS_ERR(mnt)) {
>> - ret = PTR_ERR(mnt);
>> - goto out_remove;
>> - }
>> ret = rpc_pipefs_notifier_register(&nfs4blocklayout_block);
>> if (ret)
>> goto out_remove;
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
>> the body of a message to [email protected]
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>


--
Best regards,
Stanislav Kinsbursky

2011-11-29 16:40:36

by Myklebust, Trond

[permalink] [raw]
Subject: Re: [PATCH 4/5] NFS: remove RPC PipeFS mount point reference from blocklayout routines

On Tue, 2011-11-29 at 23:30 +0800, Peng Tao wrote:
> On Tue, Nov 29, 2011 at 11:18 PM, Trond Myklebust
> <[email protected]> wrote:
> > On Tue, 2011-11-29 at 23:10 +0800, Peng Tao wrote:
> >> On Tue, Nov 29, 2011 at 9:35 PM, Myklebust, Trond
> >> <[email protected]> wrote:
> >> >> -----Original Message-----
> >> >> From: [email protected] [mailto:[email protected]]
> >> >> Sent: Tuesday, November 29, 2011 7:40 AM
> >> >> To: [email protected]
> >> >> Cc: Myklebust, Trond; [email protected]; [email protected];
> >> >> [email protected]; [email protected]; [email protected];
> >> >> [email protected]; [email protected]; [email protected];
> >> >> [email protected]
> >> >> Subject: RE: [PATCH 4/5] NFS: remove RPC PipeFS mount point reference
> >> >> from blocklayout routines
> >> >>
> >> >> > -----Original Message-----
> >> >> > From: Stanislav Kinsbursky [mailto:[email protected]]
> >> >> > Sent: Tuesday, November 29, 2011 8:19 PM
> >> >> > To: Peng, Tao
> >> >> > Cc: [email protected]; [email protected]; Pavel
> >> >> > Emelianov; [email protected]; [email protected];
> >> >> > [email protected]; James Bottomley; [email protected];
> >> >> > [email protected]; [email protected]
> >> >> > Subject: Re: [PATCH 4/5] NFS: remove RPC PipeFS mount point reference
> >> >> > from blocklayout routines
> >> >> >
> >> >> > 29.11.2011 16:00, [email protected] пишет:
> >> >> > >> -----Original Message-----
> >> >> > >> From: [email protected]
> >> >> > >> [mailto:[email protected]] On Behalf Of
> >> >> > Stanislav
> >> >> > >> Kinsbursky
> >> >> > >> Sent: Tuesday, November 29, 2011 6:11 PM
> >> >> > >> To: [email protected]
> >> >> > >> Cc: [email protected]; [email protected]; [email protected];
> >> >> > >> [email protected]; linux- [email protected];
> >> >> > >> [email protected]; [email protected];
> >> >> > >> [email protected]; [email protected]
> >> >> > >> Subject: [PATCH 4/5] NFS: remove RPC PipeFS mount point reference
> >> >> > >> from blocklayout routines
> >> >> > >>
> >> >> > >> This is a cleanup patch. We don't need this reference anymore,
> >> >> > >> because blocklayout pipes dentries now creates and destroys in
> >> >> > >> per-net operations and on PipeFS mount/umount notification.
> >> >> > >> Note that nfs4blocklayout_register_net() now returns 0 instead of
> >> >> > >> -ENOENT in case of PipeFS superblock absence. This is ok, because
> >> >> > >> blocklayout pipe dentry will be created on PipeFS mount event.
> >> >> > > When is the "pipefs mount event" going to happen? When inserting
> >> >> > > kernel modules or when user issues
> >> >> > mount command?
> >> >> > >
> >> >> >
> >> >> > When user issues mount command.
> >> >> > Kernel mounts of PipeFS has been removed with all these patch sets
> >> >> > I've sent already.
> >> >> Then it is going to break blocklayout user space program blkmapd, which is
> >> >> stared before mounting any file system and it tries to open the pipe file
> >> >> when started.
> >> >
> >> > Why on earth is blkmapd doing this instead of listening for file creation notifications like the other rpc_pipefs daemons do?
> >> Not sure how the original implementer chose this but I think it is
> >> likely because we do not expect the pipe file to be created or deleted
> >> dynamically.
> >
> > Unless blkmapd can pin the sunrpc module (which it shouldn't be able to)
> > then that assumption would be wrong. Please look into fixing blkmapd...
> Sorry, I don't quite get it. Do you mean sunrpc module may be removed
> while nfs/blocklayout modules are still in use? Please explain it a
> bit. Thanks.

I mean that I'm perfectly entitled to do

'modprobe -r blocklayoutdriver'

and when I do that, then I expect blkmapd to close the rpc pipe and wait
for a new one to be created just like rpc.idmapd and rpc.gssd do when I
remove the nfs and sunrpc modules.

--
Trond Myklebust
Linux NFS client maintainer

NetApp
[email protected]
http://www.netapp.com


2011-11-29 17:27:05

by J. Bruce Fields

[permalink] [raw]
Subject: Re: [PATCH 4/5] NFS: remove RPC PipeFS mount point reference from blocklayout routines

On Tue, Nov 29, 2011 at 12:19:25PM -0500, Trond Myklebust wrote:
> On Tue, 2011-11-29 at 11:42 -0500, J. Bruce Fields wrote:
> > On Tue, Nov 29, 2011 at 11:40:30AM -0500, Trond Myklebust wrote:
> > > I mean that I'm perfectly entitled to do
> > >
> > > 'modprobe -r blocklayoutdriver'
> > >
> > > and when I do that, then I expect blkmapd to close the rpc pipe and wait
> > > for a new one to be created just like rpc.idmapd and rpc.gssd do when I
> > > remove the nfs and sunrpc modules.
> >
> > The rpc pipefs mount doesn't hold a reference on the sunrpc module?
>
> I stand corrected: the mount does hold a reference to the sunrpc
> module.
> However nothing holds a reference to the blocklayoutdriver module, so
> the main point that the "blocklayout" pipe can disappear from underneath
> the blkmapd stands.

OK, that makes sense.

--b.

2011-11-29 09:12:21

by Stanislav Kinsbursky

[permalink] [raw]
Subject: [PATCH 2/5] NFS: blocklayout pipe creation per network namespace context introduced

This patch implements blocklayout pipe creation and registration per each
existent network namespace.
This was achived by registering NFS per-net operations, responsible for
blocklayout pipe allocation/register and unregister/destruction instead of
initialization and destruction of static "bl_device_pipe" pipe (this one was
removed).
Note, than pointer to network blocklayout pipe is stored in per-net "nfs_net"
structure, because allocating of one more per-net structure for blocklayout
module looks redundant.
This patch also changes dev_remove() function prototype (and all it's callers,
where it' requied) by adding network namespace pointer parameter, which is used
to discover proper blocklayout pipe for rpc_queue_upcall() call.

Signed-off-by: Stanislav Kinsbursky <[email protected]>

---
fs/nfs/blocklayout/blocklayout.c | 49 ++++++++++++++++++++++++-----------
fs/nfs/blocklayout/blocklayout.h | 3 +-
fs/nfs/blocklayout/blocklayoutdev.c | 5 +++-
fs/nfs/blocklayout/blocklayoutdm.c | 7 +++--
fs/nfs/inode.c | 1 +
fs/nfs/netns.h | 1 +
6 files changed, 46 insertions(+), 20 deletions(-)

diff --git a/fs/nfs/blocklayout/blocklayout.c b/fs/nfs/blocklayout/blocklayout.c
index 50d5183..bf20187 100644
--- a/fs/nfs/blocklayout/blocklayout.c
+++ b/fs/nfs/blocklayout/blocklayout.c
@@ -46,7 +46,6 @@ MODULE_LICENSE("GPL");
MODULE_AUTHOR("Andy Adamson <[email protected]>");
MODULE_DESCRIPTION("The NFSv4.1 pNFS Block layout driver");

-struct rpc_pipe *bl_device_pipe;
wait_queue_head_t bl_wq;

static void print_page(struct page *page)
@@ -1011,6 +1010,37 @@ static void nfs4blocklayout_unregister_net(struct net *net,
}
}

+static int nfs4blocklayout_net_init(struct net *net)
+{
+ struct nfs_net *nn = net_generic(net, nfs_net_id);
+ struct dentry *dentry;
+
+ nn->bl_device_pipe = rpc_mkpipe_data(&bl_upcall_ops, 0);
+ if (IS_ERR(nn->bl_device_pipe))
+ return PTR_ERR(nn->bl_device_pipe);
+ dentry = nfs4blocklayout_register_net(net, nn->bl_device_pipe);
+ if (IS_ERR(dentry)) {
+ rpc_destroy_pipe_data(nn->bl_device_pipe);
+ return PTR_ERR(dentry);
+ }
+ nn->bl_device_pipe->dentry = dentry;
+ return 0;
+}
+
+static void nfs4blocklayout_net_exit(struct net *net)
+{
+ struct nfs_net *nn = net_generic(net, nfs_net_id);
+
+ nfs4blocklayout_unregister_net(net, nn->bl_device_pipe);
+ rpc_destroy_pipe_data(nn->bl_device_pipe);
+ nn->bl_device_pipe = NULL;
+}
+
+static struct pernet_operations nfs4blocklayout_net_ops = {
+ .init = nfs4blocklayout_net_init,
+ .exit = nfs4blocklayout_net_exit,
+};
+
static int __init nfs4blocklayout_init(void)
{
struct vfsmount *mnt;
@@ -1029,22 +1059,12 @@ static int __init nfs4blocklayout_init(void)
ret = PTR_ERR(mnt);
goto out_remove;
}
- bl_device_pipe = rpc_mkpipe_data(&bl_upcall_ops, 0);
- if (IS_ERR(bl_device_pipe)) {
- ret = PTR_ERR(bl_device_pipe);
+ ret = register_pernet_subsys(&nfs4blocklayout_net_ops);
+ if (ret)
goto out_remove;
- }
- bl_device_pipe->dentry = nfs4blocklayout_register_net(&init_net,
- bl_device_pipe);
- if (IS_ERR(bl_device_pipe->dentry)) {
- ret = PTR_ERR(bl_device_pipe->dentry);
- goto out_destroy_pipe;
- }
out:
return ret;

-out_destroy_pipe:
- rpc_destroy_pipe_data(bl_device_pipe);
out_remove:
pnfs_unregister_layoutdriver(&blocklayout_type);
return ret;
@@ -1055,9 +1075,8 @@ static void __exit nfs4blocklayout_exit(void)
dprintk("%s: NFSv4 Block Layout Driver Unregistering...\n",
__func__);

+ unregister_pernet_subsys(&nfs4blocklayout_net_ops);
pnfs_unregister_layoutdriver(&blocklayout_type);
- nfs4blocklayout_unregister_net(&init_net, bl_device_pipe);
- rpc_destroy_pipe_data(bl_device_pipe);
}

MODULE_ALIAS("nfs-layouttype4-3");
diff --git a/fs/nfs/blocklayout/blocklayout.h b/fs/nfs/blocklayout/blocklayout.h
index 5f30941..059d95a 100644
--- a/fs/nfs/blocklayout/blocklayout.h
+++ b/fs/nfs/blocklayout/blocklayout.h
@@ -37,6 +37,7 @@
#include <linux/sunrpc/rpc_pipe_fs.h>

#include "../pnfs.h"
+#include "../netns.h"

#define PAGE_CACHE_SECTORS (PAGE_CACHE_SIZE >> SECTOR_SHIFT)
#define PAGE_CACHE_SECTOR_SHIFT (PAGE_CACHE_SHIFT - SECTOR_SHIFT)
@@ -50,6 +51,7 @@ struct pnfs_block_dev {
struct list_head bm_node;
struct nfs4_deviceid bm_mdevid; /* associated devid */
struct block_device *bm_mdev; /* meta device itself */
+ struct net *net;
};

enum exstate4 {
@@ -159,7 +161,6 @@ struct bl_msg_hdr {
u16 totallen; /* length of entire message, including hdr itself */
};

-extern struct rpc_pipe *bl_device_pipe;
extern wait_queue_head_t bl_wq;

#define BL_DEVICE_UMOUNT 0x0 /* Umount--delete devices */
diff --git a/fs/nfs/blocklayout/blocklayoutdev.c b/fs/nfs/blocklayout/blocklayoutdev.c
index 79f4752..8893247 100644
--- a/fs/nfs/blocklayout/blocklayoutdev.c
+++ b/fs/nfs/blocklayout/blocklayoutdev.c
@@ -142,6 +142,8 @@ nfs4_blk_decode_device(struct nfs_server *server,
DECLARE_WAITQUEUE(wq, current);
struct bl_dev_msg *reply = &bl_mount_reply;
int offset, len, i;
+ struct net *net = server->nfs_client->net;
+ struct nfs_net *nn = net_generic(net, nfs_net_id);

dprintk("%s CREATING PIPEFS MESSAGE\n", __func__);
dprintk("%s: deviceid: %s, mincount: %d\n", __func__, dev->dev_id.data,
@@ -168,7 +170,7 @@ nfs4_blk_decode_device(struct nfs_server *server,

dprintk("%s CALLING USERSPACE DAEMON\n", __func__);
add_wait_queue(&bl_wq, &wq);
- if (rpc_queue_upcall(bl_device_pipe, &msg) < 0) {
+ if (rpc_queue_upcall(nn->bl_device_pipe, &msg) < 0) {
remove_wait_queue(&bl_wq, &wq);
goto out;
}
@@ -200,6 +202,7 @@ nfs4_blk_decode_device(struct nfs_server *server,

rv->bm_mdev = bd;
memcpy(&rv->bm_mdevid, &dev->dev_id, sizeof(struct nfs4_deviceid));
+ rv->net = net;
dprintk("%s Created device %s with bd_block_size %u\n",
__func__,
bd->bd_disk->disk_name,
diff --git a/fs/nfs/blocklayout/blocklayoutdm.c b/fs/nfs/blocklayout/blocklayoutdm.c
index 631f254..970490f 100644
--- a/fs/nfs/blocklayout/blocklayoutdm.c
+++ b/fs/nfs/blocklayout/blocklayoutdm.c
@@ -38,7 +38,7 @@

#define NFSDBG_FACILITY NFSDBG_PNFS_LD

-static void dev_remove(dev_t dev)
+static void dev_remove(struct net *net, dev_t dev)
{
struct rpc_pipe_msg msg;
struct bl_dev_msg bl_umount_request;
@@ -48,6 +48,7 @@ static void dev_remove(dev_t dev)
};
uint8_t *dataptr;
DECLARE_WAITQUEUE(wq, current);
+ struct nfs_net *nn = net_generic(net, nfs_net_id);

dprintk("Entering %s\n", __func__);

@@ -66,7 +67,7 @@ static void dev_remove(dev_t dev)
msg.len = sizeof(bl_msg) + bl_msg.totallen;

add_wait_queue(&bl_wq, &wq);
- if (rpc_queue_upcall(bl_device_pipe, &msg) < 0) {
+ if (rpc_queue_upcall(nn->bl_device_pipe, &msg) < 0) {
remove_wait_queue(&bl_wq, &wq);
goto out;
}
@@ -93,7 +94,7 @@ static void nfs4_blk_metadev_release(struct pnfs_block_dev *bdev)
printk(KERN_ERR "%s nfs4_blkdev_put returns %d\n",
__func__, rv);

- dev_remove(bdev->bm_mdev->bd_dev);
+ dev_remove(bdev->net, bdev->bm_mdev->bd_dev);
}

void bl_free_block_dev(struct pnfs_block_dev *bdev)
diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c
index 84d8506..f48790c 100644
--- a/fs/nfs/inode.c
+++ b/fs/nfs/inode.c
@@ -1552,6 +1552,7 @@ static void nfsiod_stop(void)
}

int nfs_net_id;
+EXPORT_SYMBOL_GPL(nfs_net_id);

static int nfs_net_init(struct net *net)
{
diff --git a/fs/nfs/netns.h b/fs/nfs/netns.h
index 8c1f130..39ae4ca 100644
--- a/fs/nfs/netns.h
+++ b/fs/nfs/netns.h
@@ -6,6 +6,7 @@

struct nfs_net {
struct cache_detail *nfs_dns_resolve;
+ struct rpc_pipe *bl_device_pipe;
};

extern int nfs_net_id;


2011-11-29 17:19:27

by Myklebust, Trond

[permalink] [raw]
Subject: Re: [PATCH 4/5] NFS: remove RPC PipeFS mount point reference from blocklayout routines

On Tue, 2011-11-29 at 11:42 -0500, J. Bruce Fields wrote:
> On Tue, Nov 29, 2011 at 11:40:30AM -0500, Trond Myklebust wrote:
> > I mean that I'm perfectly entitled to do
> >
> > 'modprobe -r blocklayoutdriver'
> >
> > and when I do that, then I expect blkmapd to close the rpc pipe and wait
> > for a new one to be created just like rpc.idmapd and rpc.gssd do when I
> > remove the nfs and sunrpc modules.
>
> The rpc pipefs mount doesn't hold a reference on the sunrpc module?

I stand corrected: the mount does hold a reference to the sunrpc
module.
However nothing holds a reference to the blocklayoutdriver module, so
the main point that the "blocklayout" pipe can disappear from underneath
the blkmapd stands.

--
Trond Myklebust
Linux NFS client maintainer

NetApp
[email protected]
http://www.netapp.com


2011-11-29 09:12:46

by Stanislav Kinsbursky

[permalink] [raw]
Subject: [PATCH 5/5] SUNRPC: kernel PipeFS mount point creation routines removed

This patch removes static rpc_mnt variable and its creation and destruction
routines, because they are not used anymore.

Signed-off-by: Stanislav Kinsbursky <[email protected]>

---
include/linux/sunrpc/rpc_pipe_fs.h | 2 --
net/sunrpc/rpc_pipe.c | 21 ---------------------
2 files changed, 0 insertions(+), 23 deletions(-)

diff --git a/include/linux/sunrpc/rpc_pipe_fs.h b/include/linux/sunrpc/rpc_pipe_fs.h
index 7eb0160..a28d2de 100644
--- a/include/linux/sunrpc/rpc_pipe_fs.h
+++ b/include/linux/sunrpc/rpc_pipe_fs.h
@@ -90,8 +90,6 @@ void rpc_destroy_pipe_data(struct rpc_pipe *pipe);
extern struct dentry *rpc_mkpipe_dentry(struct dentry *, const char *, void *,
struct rpc_pipe *);
extern int rpc_unlink(struct dentry *);
-extern struct vfsmount *rpc_get_mount(void);
-extern void rpc_put_mount(void);
extern int register_rpc_pipefs(void);
extern void unregister_rpc_pipefs(void);

diff --git a/net/sunrpc/rpc_pipe.c b/net/sunrpc/rpc_pipe.c
index e194e32..4b1d042 100644
--- a/net/sunrpc/rpc_pipe.c
+++ b/net/sunrpc/rpc_pipe.c
@@ -18,7 +18,6 @@
#include <linux/kernel.h>

#include <asm/ioctls.h>
-#include <linux/fs.h>
#include <linux/poll.h>
#include <linux/wait.h>
#include <linux/seq_file.h>
@@ -37,9 +36,6 @@

#define NET_NAME(net) ((net == &init_net) ? " (init_net)" : "")

-static struct vfsmount *rpc_mnt __read_mostly;
-static int rpc_mount_count;
-
static struct file_system_type rpc_pipe_fs_type;


@@ -430,23 +426,6 @@ struct rpc_filelist {
umode_t mode;
};

-struct vfsmount *rpc_get_mount(void)
-{
- int err;
-
- err = simple_pin_fs(&rpc_pipe_fs_type, &rpc_mnt, &rpc_mount_count);
- if (err != 0)
- return ERR_PTR(err);
- return rpc_mnt;
-}
-EXPORT_SYMBOL_GPL(rpc_get_mount);
-
-void rpc_put_mount(void)
-{
- simple_release_fs(&rpc_mnt, &rpc_mount_count);
-}
-EXPORT_SYMBOL_GPL(rpc_put_mount);
-
static int rpc_delete_dentry(const struct dentry *dentry)
{
return 1;


2011-11-29 09:19:33

by Stanislav Kinsbursky

[permalink] [raw]
Subject: Re: [PATCH 0/5] NFS: create blocklayout pipe per network namesapce context

Hello, Trond.
This is the final part of SUNRPC PipeFS virtualization.
I hope, that you'll find some time to review all series.

You can clone my working tree to have a look at what will be at the end:

git://github.com/skinsbursky/nfs-per-net-ns.git

BTW, I can provide a simple "sandbox" (a kind of container with it's own network
namespace and veth device inside) which I use to test my changes.

--
Best regards,
Stanislav Kinsbursky

2011-11-29 09:12:36

by Stanislav Kinsbursky

[permalink] [raw]
Subject: [PATCH 4/5] NFS: remove RPC PipeFS mount point reference from blocklayout routines

This is a cleanup patch. We don't need this reference anymore, because
blocklayout pipes dentries now creates and destroys in per-net operations and
on PipeFS mount/umount notification.
Note that nfs4blocklayout_register_net() now returns 0 instead of -ENOENT in
case of PipeFS superblock absence. This is ok, because blocklayout pipe dentry
will be created on PipeFS mount event.

Signed-off-by: Stanislav Kinsbursky <[email protected]>

---
fs/nfs/blocklayout/blocklayout.c | 9 +--------
1 files changed, 1 insertions(+), 8 deletions(-)

diff --git a/fs/nfs/blocklayout/blocklayout.c b/fs/nfs/blocklayout/blocklayout.c
index acf7ac9..8211ffd 100644
--- a/fs/nfs/blocklayout/blocklayout.c
+++ b/fs/nfs/blocklayout/blocklayout.c
@@ -1032,7 +1032,7 @@ static struct dentry *nfs4blocklayout_register_net(struct net *net,

pipefs_sb = rpc_get_sb_net(net);
if (!pipefs_sb)
- return ERR_PTR(-ENOENT);
+ return 0;
dentry = nfs4blocklayout_register_sb(pipefs_sb, pipe);
rpc_put_sb_net(net);
return dentry;
@@ -1083,7 +1083,6 @@ static struct pernet_operations nfs4blocklayout_net_ops = {

static int __init nfs4blocklayout_init(void)
{
- struct vfsmount *mnt;
int ret;

dprintk("%s: NFSv4 Block Layout Driver Registering...\n", __func__);
@@ -1093,12 +1092,6 @@ static int __init nfs4blocklayout_init(void)
goto out;

init_waitqueue_head(&bl_wq);
-
- mnt = rpc_get_mount();
- if (IS_ERR(mnt)) {
- ret = PTR_ERR(mnt);
- goto out_remove;
- }
ret = rpc_pipefs_notifier_register(&nfs4blocklayout_block);
if (ret)
goto out_remove;


2011-11-29 12:01:37

by Peng, Tao

[permalink] [raw]
Subject: RE: [PATCH 4/5] NFS: remove RPC PipeFS mount point reference from blocklayout routines

PiAtLS0tLU9yaWdpbmFsIE1lc3NhZ2UtLS0tLQ0KPiBGcm9tOiBsaW51eC1uZnMtb3duZXJAdmdl
ci5rZXJuZWwub3JnIFttYWlsdG86bGludXgtbmZzLW93bmVyQHZnZXIua2VybmVsLm9yZ10gT24g
QmVoYWxmIE9mIFN0YW5pc2xhdg0KPiBLaW5zYnVyc2t5DQo+IFNlbnQ6IFR1ZXNkYXksIE5vdmVt
YmVyIDI5LCAyMDExIDY6MTEgUE0NCj4gVG86IFRyb25kLk15a2xlYnVzdEBuZXRhcHAuY29tDQo+
IENjOiBsaW51eC1uZnNAdmdlci5rZXJuZWwub3JnOyB4ZW11bEBwYXJhbGxlbHMuY29tOyBuZWls
YkBzdXNlLmRlOyBuZXRkZXZAdmdlci5rZXJuZWwub3JnOyBsaW51eC0NCj4ga2VybmVsQHZnZXIu
a2VybmVsLm9yZzsgamJvdHRvbWxleUBwYXJhbGxlbHMuY29tOyBiZmllbGRzQGZpZWxkc2VzLm9y
ZzsgZGF2ZW1AZGF2ZW1sb2Z0Lm5ldDsNCj4gZGV2ZWxAb3BlbnZ6Lm9yZw0KPiBTdWJqZWN0OiBb
UEFUQ0ggNC81XSBORlM6IHJlbW92ZSBSUEMgUGlwZUZTIG1vdW50IHBvaW50IHJlZmVyZW5jZSBm
cm9tIGJsb2NrbGF5b3V0IHJvdXRpbmVzDQo+IA0KPiBUaGlzIGlzIGEgY2xlYW51cCBwYXRjaC4g
V2UgZG9uJ3QgbmVlZCB0aGlzIHJlZmVyZW5jZSBhbnltb3JlLCBiZWNhdXNlDQo+IGJsb2NrbGF5
b3V0IHBpcGVzIGRlbnRyaWVzIG5vdyBjcmVhdGVzIGFuZCBkZXN0cm95cyBpbiBwZXItbmV0IG9w
ZXJhdGlvbnMgYW5kDQo+IG9uIFBpcGVGUyBtb3VudC91bW91bnQgbm90aWZpY2F0aW9uLg0KPiBO
b3RlIHRoYXQgbmZzNGJsb2NrbGF5b3V0X3JlZ2lzdGVyX25ldCgpIG5vdyByZXR1cm5zIDAgaW5z
dGVhZCBvZiAtRU5PRU5UIGluDQo+IGNhc2Ugb2YgUGlwZUZTIHN1cGVyYmxvY2sgYWJzZW5jZS4g
VGhpcyBpcyBvaywgYmVjYXVzZSBibG9ja2xheW91dCBwaXBlIGRlbnRyeQ0KPiB3aWxsIGJlIGNy
ZWF0ZWQgb24gUGlwZUZTIG1vdW50IGV2ZW50Lg0KV2hlbiBpcyB0aGUgInBpcGVmcyBtb3VudCBl
dmVudCIgZ29pbmcgdG8gaGFwcGVuPyBXaGVuIGluc2VydGluZyBrZXJuZWwgbW9kdWxlcyBvciB3
aGVuIHVzZXIgaXNzdWVzIG1vdW50IGNvbW1hbmQ/DQoNClRoYW5rcywNClRhbw0KDQo+IA0KPiBT
aWduZWQtb2ZmLWJ5OiBTdGFuaXNsYXYgS2luc2J1cnNreSA8c2tpbnNidXJza3lAcGFyYWxsZWxz
LmNvbT4NCj4gDQo+IC0tLQ0KPiAgZnMvbmZzL2Jsb2NrbGF5b3V0L2Jsb2NrbGF5b3V0LmMgfCAg
ICA5ICstLS0tLS0tLQ0KPiAgMSBmaWxlcyBjaGFuZ2VkLCAxIGluc2VydGlvbnMoKyksIDggZGVs
ZXRpb25zKC0pDQo+IA0KPiBkaWZmIC0tZ2l0IGEvZnMvbmZzL2Jsb2NrbGF5b3V0L2Jsb2NrbGF5
b3V0LmMgYi9mcy9uZnMvYmxvY2tsYXlvdXQvYmxvY2tsYXlvdXQuYw0KPiBpbmRleCBhY2Y3YWM5
Li44MjExZmZkIDEwMDY0NA0KPiAtLS0gYS9mcy9uZnMvYmxvY2tsYXlvdXQvYmxvY2tsYXlvdXQu
Yw0KPiArKysgYi9mcy9uZnMvYmxvY2tsYXlvdXQvYmxvY2tsYXlvdXQuYw0KPiBAQCAtMTAzMiw3
ICsxMDMyLDcgQEAgc3RhdGljIHN0cnVjdCBkZW50cnkgKm5mczRibG9ja2xheW91dF9yZWdpc3Rl
cl9uZXQoc3RydWN0IG5ldCAqbmV0LA0KPiANCj4gIAlwaXBlZnNfc2IgPSBycGNfZ2V0X3NiX25l
dChuZXQpOw0KPiAgCWlmICghcGlwZWZzX3NiKQ0KPiAtCQlyZXR1cm4gRVJSX1BUUigtRU5PRU5U
KTsNCj4gKwkJcmV0dXJuIDA7DQo+ICAJZGVudHJ5ID0gbmZzNGJsb2NrbGF5b3V0X3JlZ2lzdGVy
X3NiKHBpcGVmc19zYiwgcGlwZSk7DQo+ICAJcnBjX3B1dF9zYl9uZXQobmV0KTsNCj4gIAlyZXR1
cm4gZGVudHJ5Ow0KPiBAQCAtMTA4Myw3ICsxMDgzLDYgQEAgc3RhdGljIHN0cnVjdCBwZXJuZXRf
b3BlcmF0aW9ucyBuZnM0YmxvY2tsYXlvdXRfbmV0X29wcyA9IHsNCj4gDQo+ICBzdGF0aWMgaW50
IF9faW5pdCBuZnM0YmxvY2tsYXlvdXRfaW5pdCh2b2lkKQ0KPiAgew0KPiAtCXN0cnVjdCB2ZnNt
b3VudCAqbW50Ow0KPiAgCWludCByZXQ7DQo+IA0KPiAgCWRwcmludGsoIiVzOiBORlN2NCBCbG9j
ayBMYXlvdXQgRHJpdmVyIFJlZ2lzdGVyaW5nLi4uXG4iLCBfX2Z1bmNfXyk7DQo+IEBAIC0xMDkz
LDEyICsxMDkyLDYgQEAgc3RhdGljIGludCBfX2luaXQgbmZzNGJsb2NrbGF5b3V0X2luaXQodm9p
ZCkNCj4gIAkJZ290byBvdXQ7DQo+IA0KPiAgCWluaXRfd2FpdHF1ZXVlX2hlYWQoJmJsX3dxKTsN
Cj4gLQ0KPiAtCW1udCA9IHJwY19nZXRfbW91bnQoKTsNCj4gLQlpZiAoSVNfRVJSKG1udCkpIHsN
Cj4gLQkJcmV0ID0gUFRSX0VSUihtbnQpOw0KPiAtCQlnb3RvIG91dF9yZW1vdmU7DQo+IC0JfQ0K
PiAgCXJldCA9IHJwY19waXBlZnNfbm90aWZpZXJfcmVnaXN0ZXIoJm5mczRibG9ja2xheW91dF9i
bG9jayk7DQo+ICAJaWYgKHJldCkNCj4gIAkJZ290byBvdXRfcmVtb3ZlOw0KPiANCj4gLS0NCj4g
VG8gdW5zdWJzY3JpYmUgZnJvbSB0aGlzIGxpc3Q6IHNlbmQgdGhlIGxpbmUgInVuc3Vic2NyaWJl
IGxpbnV4LW5mcyIgaW4NCj4gdGhlIGJvZHkgb2YgYSBtZXNzYWdlIHRvIG1ham9yZG9tb0B2Z2Vy
Lmtlcm5lbC5vcmcNCj4gTW9yZSBtYWpvcmRvbW8gaW5mbyBhdCAgaHR0cDovL3ZnZXIua2VybmVs
Lm9yZy9tYWpvcmRvbW8taW5mby5odG1sDQoNCg==

2011-11-29 15:10:38

by Peng Tao

[permalink] [raw]
Subject: Re: [PATCH 4/5] NFS: remove RPC PipeFS mount point reference from blocklayout routines

On Tue, Nov 29, 2011 at 9:35 PM, Myklebust, Trond
<[email protected]> wrote:
>> -----Original Message-----
>> From: [email protected] [mailto:[email protected]]
>> Sent: Tuesday, November 29, 2011 7:40 AM
>> To: [email protected]
>> Cc: Myklebust, Trond; [email protected]; [email protected];
>> [email protected]; [email protected]; [email protected];
>> [email protected]; [email protected]; [email protected];
>> [email protected]
>> Subject: RE: [PATCH 4/5] NFS: remove RPC PipeFS mount point reference
>> from blocklayout routines
>>
>> > -----Original Message-----
>> > From: Stanislav Kinsbursky [mailto:[email protected]]
>> > Sent: Tuesday, November 29, 2011 8:19 PM
>> > To: Peng, Tao
>> > Cc: [email protected]; [email protected]; Pavel
>> > Emelianov; [email protected]; [email protected];
>> > [email protected]; James Bottomley; [email protected];
>> > [email protected]; [email protected]
>> > Subject: Re: [PATCH 4/5] NFS: remove RPC PipeFS mount point reference
>> > from blocklayout routines
>> >
>> > 29.11.2011 16:00, [email protected] пишет:
>> > >> -----Original Message-----
>> > >> From: [email protected]
>> > >> [mailto:[email protected]] On Behalf Of
>> > Stanislav
>> > >> Kinsbursky
>> > >> Sent: Tuesday, November 29, 2011 6:11 PM
>> > >> To: [email protected]
>> > >> Cc: [email protected]; [email protected]; [email protected];
>> > >> [email protected]; linux- [email protected];
>> > >> [email protected]; [email protected];
>> > >> [email protected]; [email protected]
>> > >> Subject: [PATCH 4/5] NFS: remove RPC PipeFS mount point reference
>> > >> from blocklayout routines
>> > >>
>> > >> This is a cleanup patch. We don't need this reference anymore,
>> > >> because blocklayout pipes dentries now creates and destroys in
>> > >> per-net operations and on PipeFS mount/umount notification.
>> > >> Note that nfs4blocklayout_register_net() now returns 0 instead of
>> > >> -ENOENT in case of PipeFS superblock absence. This is ok, because
>> > >> blocklayout pipe dentry will be created on PipeFS mount event.
>> > > When is the "pipefs mount event" going to happen? When inserting
>> > > kernel modules or when user issues
>> > mount command?
>> > >
>> >
>> > When user issues mount command.
>> > Kernel mounts of PipeFS has been removed with all these patch sets
>> > I've sent already.
>> Then it is going to break blocklayout user space program blkmapd, which is
>> stared before mounting any file system and it tries to open the pipe file
>> when started.
>
> Why on earth is blkmapd doing this instead of listening for file creation notifications like the other rpc_pipefs daemons do?
Not sure how the original implementer chose this but I think it is
likely because we do not expect the pipe file to be created or deleted
dynamically.

--
Thanks,
Tao

2011-11-29 09:12:30

by Stanislav Kinsbursky

[permalink] [raw]
Subject: [PATCH 3/5] NFS: blocklayout PipeFS notifier introduced

This patch subscribes blocklayout pipes to RPC pipefs notifications. Notifier
is registering on blocklayout module load. This notifier callback is
responsible for creation/destruction of PipeFS blocklayout pipe dentry.
Note that no locking required in notifier callback because PipeFS superblock
pointer is passed as an argument from it's creation or destruction routine and
thus we can be sure about it's validity.

Signed-off-by: Stanislav Kinsbursky <[email protected]>

---
fs/nfs/blocklayout/blocklayout.c | 48 +++++++++++++++++++++++++++++++++++++-
1 files changed, 47 insertions(+), 1 deletions(-)

diff --git a/fs/nfs/blocklayout/blocklayout.c b/fs/nfs/blocklayout/blocklayout.c
index bf20187..acf7ac9 100644
--- a/fs/nfs/blocklayout/blocklayout.c
+++ b/fs/nfs/blocklayout/blocklayout.c
@@ -984,6 +984,46 @@ static void nfs4blocklayout_unregister_sb(struct super_block *sb,
rpc_unlink(pipe->dentry);
}

+static int rpc_pipefs_event(struct notifier_block *nb, unsigned long event,
+ void *ptr)
+{
+ struct super_block *sb = ptr;
+ struct net *net = sb->s_fs_info;
+ struct nfs_net *nn = net_generic(net, nfs_net_id);
+ struct dentry *dentry;
+ int ret = 0;
+
+ if (!try_module_get(THIS_MODULE))
+ return 0;
+
+ if (nn->bl_device_pipe == NULL)
+ return 0;
+
+ switch (event) {
+ case RPC_PIPEFS_MOUNT:
+ dentry = nfs4blocklayout_register_sb(sb, nn->bl_device_pipe);
+ if (IS_ERR(dentry)) {
+ ret = PTR_ERR(dentry);
+ break;
+ }
+ nn->bl_device_pipe->dentry = dentry;
+ break;
+ case RPC_PIPEFS_UMOUNT:
+ if (nn->bl_device_pipe->dentry)
+ nfs4blocklayout_unregister_sb(sb, nn->bl_device_pipe);
+ break;
+ default:
+ ret = -ENOTSUPP;
+ break;
+ }
+ module_put(THIS_MODULE);
+ return ret;
+}
+
+static struct notifier_block nfs4blocklayout_block = {
+ .notifier_call = rpc_pipefs_event,
+};
+
static struct dentry *nfs4blocklayout_register_net(struct net *net,
struct rpc_pipe *pipe)
{
@@ -1059,12 +1099,17 @@ static int __init nfs4blocklayout_init(void)
ret = PTR_ERR(mnt);
goto out_remove;
}
- ret = register_pernet_subsys(&nfs4blocklayout_net_ops);
+ ret = rpc_pipefs_notifier_register(&nfs4blocklayout_block);
if (ret)
goto out_remove;
+ ret = register_pernet_subsys(&nfs4blocklayout_net_ops);
+ if (ret)
+ goto out_notifier;
out:
return ret;

+out_notifier:
+ rpc_pipefs_notifier_unregister(&nfs4blocklayout_block);
out_remove:
pnfs_unregister_layoutdriver(&blocklayout_type);
return ret;
@@ -1075,6 +1120,7 @@ static void __exit nfs4blocklayout_exit(void)
dprintk("%s: NFSv4 Block Layout Driver Unregistering...\n",
__func__);

+ rpc_pipefs_notifier_unregister(&nfs4blocklayout_block);
unregister_pernet_subsys(&nfs4blocklayout_net_ops);
pnfs_unregister_layoutdriver(&blocklayout_type);
}


2011-12-30 22:55:06

by Myklebust, Trond

[permalink] [raw]
Subject: Re: [PATCH 0/5] NFS: create blocklayout pipe per network namesapce context

On Tue, 2011-11-29 at 13:10 +0300, Stanislav Kinsbursky wrote:
> This patch set was created in context of clone of git
> branch: git://git.linux-nfs.org/projects/trondmy/nfs-2.6.git.
> tag: v3.1
>
> This patch set depends on previous patch sets titled:
> 1) "SUNRPC: initial part of making pipefs work in net ns"
> 2) "SUNPRC: cleanup PipeFS for network-namespace-aware users"
> 3) "SUNRPC: make RPC clients use network-namespace-aware PipeFS routines"
> 4) "NFS: create clients and IDMAP pipes per network namespace"
>
> This patch set is the final part of making SUNRPC PipeFS and it's users work in
> network namespace context.
>
> The following series consists of:
>
> ---
>
> Stanislav Kinsbursky (5):
> NFS: handle blocklayout pipe PipeFS dentry by network namespace aware routines
> NFS: blocklayout pipe creation per network namespace context introduced
> NFS: blocklayout PipeFS notifier introduced
> NFS: remove RPC PipeFS mount point reference from blocklayout routines
> SUNRPC: kernel PipeFS mount point creation routines removed
>
>
> fs/nfs/blocklayout/blocklayout.c | 154 ++++++++++++++++++++++++++++-------
> fs/nfs/blocklayout/blocklayout.h | 3 -
> fs/nfs/blocklayout/blocklayoutdev.c | 5 +
> fs/nfs/blocklayout/blocklayoutdm.c | 7 +-
> fs/nfs/inode.c | 1
> fs/nfs/netns.h | 1
> include/linux/sunrpc/rpc_pipe_fs.h | 2
> net/sunrpc/rpc_pipe.c | 21 -----
> 8 files changed, 137 insertions(+), 57 deletions(-)


These patches need rebasing in order to apply on top of 3.2-rc7...

Cheers
Trond

--
Trond Myklebust
Linux NFS client maintainer

NetApp
[email protected]
http://www.netapp.com


2012-01-11 17:46:07

by Myklebust, Trond

[permalink] [raw]
Subject: Re: [PATCH 0/5] NFS: create blocklayout pipe per network namesapce context

On Wed, 2012-01-11 at 21:23 +0400, Stanislav Kinsbursky wrote:
> I've sent rebased "v2" of the patch set, contains updated patch "SUNRPC: handle
> RPC client pipefs dentries by network namespace aware routine", which, I
> believe, fixes oops, spotted by Bryan (it was caused by excessive call of
> rpc_put_mount() on PipeFS dentries unlink).
> So, if I'm not mistaken here, there's no need in implementing of dummy versions
> of rpc_pipefs_notifier_(un)register() or any other dummy stuff.

OK. Fair enough. We'll give it some testing to make sure that we don't
hit it again...

> BTW, it looks like that in last 2 days I've sent all updates to the issues you
> pointed out. If not, please, ping me once more.

I'm in the process of reviewing it right now.

Cheers
Trond

--
Trond Myklebust
Linux NFS client maintainer

NetApp
[email protected]
http://www.netapp.com


2012-01-10 10:50:30

by Stanislav Kinsbursky

[permalink] [raw]
Subject: Re: [PATCH 0/5] NFS: create blocklayout pipe per network namesapce context

31.12.2011 02:55, Trond Myklebust пишет:
> On Tue, 2011-11-29 at 13:10 +0300, Stanislav Kinsbursky wrote:
>> This patch set was created in context of clone of git
>> branch: git://git.linux-nfs.org/projects/trondmy/nfs-2.6.git.
>> tag: v3.1
>>
>> This patch set depends on previous patch sets titled:
>> 1) "SUNRPC: initial part of making pipefs work in net ns"
>> 2) "SUNPRC: cleanup PipeFS for network-namespace-aware users"
>> 3) "SUNRPC: make RPC clients use network-namespace-aware PipeFS routines"
>> 4) "NFS: create clients and IDMAP pipes per network namespace"
>>
>> This patch set is the final part of making SUNRPC PipeFS and it's users work in
>> network namespace context.
>>
>> The following series consists of:
>>
>> ---
>>
>> Stanislav Kinsbursky (5):
>> NFS: handle blocklayout pipe PipeFS dentry by network namespace aware routines
>> NFS: blocklayout pipe creation per network namespace context introduced
>> NFS: blocklayout PipeFS notifier introduced
>> NFS: remove RPC PipeFS mount point reference from blocklayout routines
>> SUNRPC: kernel PipeFS mount point creation routines removed
>>
>>
>> fs/nfs/blocklayout/blocklayout.c | 154 ++++++++++++++++++++++++++++-------
>> fs/nfs/blocklayout/blocklayout.h | 3 -
>> fs/nfs/blocklayout/blocklayoutdev.c | 5 +
>> fs/nfs/blocklayout/blocklayoutdm.c | 7 +-
>> fs/nfs/inode.c | 1
>> fs/nfs/netns.h | 1
>> include/linux/sunrpc/rpc_pipe_fs.h | 2
>> net/sunrpc/rpc_pipe.c | 21 -----
>> 8 files changed, 137 insertions(+), 57 deletions(-)
>
>
> These patches need rebasing in order to apply on top of 3.2-rc7...
>

Will resend rebased version soon.

--
Best regards,
Stanislav Kinsbursky

2012-01-05 20:58:59

by Myklebust, Trond

[permalink] [raw]
Subject: Re: [PATCH 0/5] NFS: create blocklayout pipe per network namesapce context

On Fri, 2011-12-30 at 17:55 -0500, Trond Myklebust wrote:
> On Tue, 2011-11-29 at 13:10 +0300, Stanislav Kinsbursky wrote:
> > This patch set was created in context of clone of git
> > branch: git://git.linux-nfs.org/projects/trondmy/nfs-2.6.git.
> > tag: v3.1
> >
> > This patch set depends on previous patch sets titled:
> > 1) "SUNRPC: initial part of making pipefs work in net ns"
> > 2) "SUNPRC: cleanup PipeFS for network-namespace-aware users"
> > 3) "SUNRPC: make RPC clients use network-namespace-aware PipeFS routines"
> > 4) "NFS: create clients and IDMAP pipes per network namespace"
> >
> > This patch set is the final part of making SUNRPC PipeFS and it's users work in
> > network namespace context.
> >
> > The following series consists of:
> >
> > ---
> >
> > Stanislav Kinsbursky (5):
> > NFS: handle blocklayout pipe PipeFS dentry by network namespace aware routines
> > NFS: blocklayout pipe creation per network namespace context introduced
> > NFS: blocklayout PipeFS notifier introduced
> > NFS: remove RPC PipeFS mount point reference from blocklayout routines
> > SUNRPC: kernel PipeFS mount point creation routines removed
> >
> >
> > fs/nfs/blocklayout/blocklayout.c | 154 ++++++++++++++++++++++++++++-------
> > fs/nfs/blocklayout/blocklayout.h | 3 -
> > fs/nfs/blocklayout/blocklayoutdev.c | 5 +
> > fs/nfs/blocklayout/blocklayoutdm.c | 7 +-
> > fs/nfs/inode.c | 1
> > fs/nfs/netns.h | 1
> > include/linux/sunrpc/rpc_pipe_fs.h | 2
> > net/sunrpc/rpc_pipe.c | 21 -----
> > 8 files changed, 137 insertions(+), 57 deletions(-)
>
>
> These patches need rebasing in order to apply on top of 3.2-rc7...

OK. Further testing seems to indicate that we're going to have to
postpone merging these patches until the 3.4 merge window.

The problems are twofold:

As the patches stand now in the linux-next tree, they can (and
occasionally do) Oops on unmount. The reason was that I rejected the
PipeFS notifier patch for the idmapper (due to the reported problem of
nfs_idmap_init/nfs_idmap_quit being undefined when CONFIG_NFS_V4 is
undefined), and the fact that it is missing causes the unmount at the
end of our tests to hit the BUG() in fs/dcache.c:905. This suggests that
we will have the same problem with the pNFS block layout driver, since I
still haven't received a rebased update of the 5 'create blocklayout
pipe per network namesapce context' patches.

The second problem that was highlighted was the fact that as they stand
today, these patchsets do not allow for bisection. When we hit the Oops,
I had Bryan try to bisect where the problem arose. He ended up pointing
at the patch "SUNRPC: handle RPC client pipefs dentries by network
namespace aware routine", which is indeed the cause, but which is one of
the _dependencies_ for all the PipeFS notifier patches that fix the
problem.

--
Trond Myklebust
Linux NFS client maintainer

NetApp
[email protected]
http://www.netapp.com


2012-01-10 12:58:45

by Stanislav Kinsbursky

[permalink] [raw]
Subject: Re: [PATCH 0/5] NFS: create blocklayout pipe per network namesapce context

06.01.2012 00:58, Trond Myklebust пишет:
> On Fri, 2011-12-30 at 17:55 -0500, Trond Myklebust wrote:
>> On Tue, 2011-11-29 at 13:10 +0300, Stanislav Kinsbursky wrote:
>>> This patch set was created in context of clone of git
>>> branch: git://git.linux-nfs.org/projects/trondmy/nfs-2.6.git.
>>> tag: v3.1
>>>
>>> This patch set depends on previous patch sets titled:
>>> 1) "SUNRPC: initial part of making pipefs work in net ns"
>>> 2) "SUNPRC: cleanup PipeFS for network-namespace-aware users"
>>> 3) "SUNRPC: make RPC clients use network-namespace-aware PipeFS routines"
>>> 4) "NFS: create clients and IDMAP pipes per network namespace"
>>>
>>> This patch set is the final part of making SUNRPC PipeFS and it's users work in
>>> network namespace context.
>>>
>>> The following series consists of:
>>>
>>> ---
>>>
>>> Stanislav Kinsbursky (5):
>>> NFS: handle blocklayout pipe PipeFS dentry by network namespace aware routines
>>> NFS: blocklayout pipe creation per network namespace context introduced
>>> NFS: blocklayout PipeFS notifier introduced
>>> NFS: remove RPC PipeFS mount point reference from blocklayout routines
>>> SUNRPC: kernel PipeFS mount point creation routines removed
>>>
>>>
>>> fs/nfs/blocklayout/blocklayout.c | 154 ++++++++++++++++++++++++++++-------
>>> fs/nfs/blocklayout/blocklayout.h | 3 -
>>> fs/nfs/blocklayout/blocklayoutdev.c | 5 +
>>> fs/nfs/blocklayout/blocklayoutdm.c | 7 +-
>>> fs/nfs/inode.c | 1
>>> fs/nfs/netns.h | 1
>>> include/linux/sunrpc/rpc_pipe_fs.h | 2
>>> net/sunrpc/rpc_pipe.c | 21 -----
>>> 8 files changed, 137 insertions(+), 57 deletions(-)
>>
>>
>> These patches need rebasing in order to apply on top of 3.2-rc7...
>
> OK. Further testing seems to indicate that we're going to have to
> postpone merging these patches until the 3.4 merge window.
>
> The problems are twofold:
>
> As the patches stand now in the linux-next tree, they can (and
> occasionally do) Oops on unmount. The reason was that I rejected the
> PipeFS notifier patch for the idmapper (due to the reported problem of
> nfs_idmap_init/nfs_idmap_quit being undefined when CONFIG_NFS_V4 is
> undefined), and the fact that it is missing causes the unmount at the
> end of our tests to hit the BUG() in fs/dcache.c:905. This suggests that
> we will have the same problem with the pNFS block layout driver, since I
> still haven't received a rebased update of the 5 'create blocklayout
> pipe per network namesapce context' patches.
>

Hello, Trond.
I've resend the patch set (rebased with fix for nfs_idmap_init/nfs_idmap_quit).

> The second problem that was highlighted was the fact that as they stand
> today, these patchsets do not allow for bisection. When we hit the Oops,
> I had Bryan try to bisect where the problem arose. He ended up pointing
> at the patch "SUNRPC: handle RPC client pipefs dentries by network
> namespace aware routine", which is indeed the cause, but which is one of
> the _dependencies_ for all the PipeFS notifier patches that fix the
> problem.
>

I'm confused here. Does this means, that I have to fix patch "SUNRPC: handle RPC
client pipefs dentries by network namespace aware routine" to make it able to
bisect?

--
Best regards,
Stanislav Kinsbursky

2012-01-11 16:23:26

by Myklebust, Trond

[permalink] [raw]
Subject: Re: [PATCH 0/5] NFS: create blocklayout pipe per network namesapce context

On Tue, 2012-01-10 at 16:58 +0400, Stanislav Kinsbursky wrote:
> 06.01.2012 00:58, Trond Myklebust пишет:
> > The second problem that was highlighted was the fact that as they stand
> > today, these patchsets do not allow for bisection. When we hit the Oops,
> > I had Bryan try to bisect where the problem arose. He ended up pointing
> > at the patch "SUNRPC: handle RPC client pipefs dentries by network
> > namespace aware routine", which is indeed the cause, but which is one of
> > the _dependencies_ for all the PipeFS notifier patches that fix the
> > problem.
> >
>
> I'm confused here. Does this means, that I have to fix patch "SUNRPC: handle RPC
> client pipefs dentries by network namespace aware routine" to make it able to
> bisect?

What I mean is that currently, I have various ways to Oops the kernel
when I apply "SUNRPC: handle RPC client pipefs dentries by network
namespace aware routine" before all these other followup patches are
applied.

One way to could fix this, might be to add dummy versions of
rpc_pipefs_notifier_register()/unregister() so that "NFS: idmap PipeFS
notifier introduced" and the other such patches can be applied without
compilation errors or Oopses before the "handle RPC client pipefs
dentries..." patch is applied. The latter could then enable the real
rpc_pipefs_notifier_register()/....

The point is to not have these patches add _known_ bugs to the kernel at
any point, so that someone who is trying to track down an unknown bug
via "git bisect" doesn't have to also cope with these avoidable
issues...

--
Trond Myklebust
Linux NFS client maintainer

NetApp
[email protected]
http://www.netapp.com


2012-01-11 18:03:39

by Stanislav Kinsbursky

[permalink] [raw]
Subject: Re: [PATCH 0/5] NFS: create blocklayout pipe per network namesapce context

11.01.2012 21:46, Trond Myklebust пишет:
> On Wed, 2012-01-11 at 21:23 +0400, Stanislav Kinsbursky wrote:
>> I've sent rebased "v2" of the patch set, contains updated patch "SUNRPC: handle
>> RPC client pipefs dentries by network namespace aware routine", which, I
>> believe, fixes oops, spotted by Bryan (it was caused by excessive call of
>> rpc_put_mount() on PipeFS dentries unlink).
>> So, if I'm not mistaken here, there's no need in implementing of dummy versions
>> of rpc_pipefs_notifier_(un)register() or any other dummy stuff.
>
> OK. Fair enough. We'll give it some testing to make sure that we don't
> hit it again...
>
>> BTW, it looks like that in last 2 days I've sent all updates to the issues you
>> pointed out. If not, please, ping me once more.
>
> I'm in the process of reviewing it right now.
>

Cool, thanks, Trond.

> Cheers
> Trond
>


--
Best regards,
Stanislav Kinsbursky

2012-01-11 17:23:37

by Stanislav Kinsbursky

[permalink] [raw]
Subject: Re: [PATCH 0/5] NFS: create blocklayout pipe per network namesapce context

11.01.2012 20:23, Trond Myklebust пишет:
> On Tue, 2012-01-10 at 16:58 +0400, Stanislav Kinsbursky wrote:
>> 06.01.2012 00:58, Trond Myklebust пишет:
>>> The second problem that was highlighted was the fact that as they stand
>>> today, these patchsets do not allow for bisection. When we hit the Oops,
>>> I had Bryan try to bisect where the problem arose. He ended up pointing
>>> at the patch "SUNRPC: handle RPC client pipefs dentries by network
>>> namespace aware routine", which is indeed the cause, but which is one of
>>> the _dependencies_ for all the PipeFS notifier patches that fix the
>>> problem.
>>>
>>
>> I'm confused here. Does this means, that I have to fix patch "SUNRPC: handle RPC
>> client pipefs dentries by network namespace aware routine" to make it able to
>> bisect?
>
> What I mean is that currently, I have various ways to Oops the kernel
> when I apply "SUNRPC: handle RPC client pipefs dentries by network
> namespace aware routine" before all these other followup patches are
> applied.
>
> One way to could fix this, might be to add dummy versions of
> rpc_pipefs_notifier_register()/unregister() so that "NFS: idmap PipeFS
> notifier introduced" and the other such patches can be applied without
> compilation errors or Oopses before the "handle RPC client pipefs
> dentries..." patch is applied. The latter could then enable the real
> rpc_pipefs_notifier_register()/....
>
> The point is to not have these patches add _known_ bugs to the kernel at
> any point, so that someone who is trying to track down an unknown bug
> via "git bisect" doesn't have to also cope with these avoidable
> issues...
>

Ok, thanks for explanation.
I've sent rebased "v2" of the patch set, contains updated patch "SUNRPC: handle
RPC client pipefs dentries by network namespace aware routine", which, I
believe, fixes oops, spotted by Bryan (it was caused by excessive call of
rpc_put_mount() on PipeFS dentries unlink).
So, if I'm not mistaken here, there's no need in implementing of dummy versions
of rpc_pipefs_notifier_(un)register() or any other dummy stuff.

BTW, it looks like that in last 2 days I've sent all updates to the issues you
pointed out. If not, please, ping me once more.

--
Best regards,
Stanislav Kinsbursky

2012-05-28 11:44:50

by Boaz Harrosh

[permalink] [raw]
Subject: Re: [PATCH 4/5] NFS: remove RPC PipeFS mount point reference from blocklayout routines

On 11/29/2011 07:30 PM, Peng Tao wrote:

> On Wed, Nov 30, 2011 at 1:19 AM, Trond Myklebust
> <[email protected]> wrote:
>> On Tue, 2011-11-29 at 11:42 -0500, J. Bruce Fields wrote:
>>> On Tue, Nov 29, 2011 at 11:40:30AM -0500, Trond Myklebust wrote:
>>>> I mean that I'm perfectly entitled to do
>>>>
>>>> 'modprobe -r blocklayoutdriver'
>>>>
>>>> and when I do that, then I expect blkmapd to close the rpc pipe and wait
>>>> for a new one to be created just like rpc.idmapd and rpc.gssd do when I
>>>> remove the nfs and sunrpc modules.
>>>
>>> The rpc pipefs mount doesn't hold a reference on the sunrpc module?
>>
>> I stand corrected: the mount does hold a reference to the sunrpc
>> module.
>> However nothing holds a reference to the blocklayoutdriver module, so
>> the main point that the "blocklayout" pipe can disappear from underneath
>> the blkmapd stands.
> Thanks for the explanation and I agree it can cause problem if user
> reload blocklayout module. I will look into a fix to blkmapd.
>


You might want to consider converting to call_usermodehelper()

I know that it greatly simplified our code both in Kernel and
in user-mode. And it made nfs-utils maintainer much happier
as well.

The speed is not Cardinal here I think. Like in objects it's
done once per new device_id

> Best,
> Tao


Just my $0.017
Boaz