NFS bdi flush thread in ps output is printed like "flush-<major number
in decimal>:<minor number in decimal>"
For example:
$ ps aux | grep flush
2079 root 0 SW [flush-0:18]
^^^^
nfs_bdi_register()
==> bdi_register_dev()
==> bdi_register(bdi, NULL, "%u:%u", MAJOR(dev), MINOR(dev));
^^^^^
However, NFS sb->s_id store major:minor number in hex:
nfs_initialise_sb()
==> snprintf(sb->s_id, sizeof(sb->s_id),
"%x:%x", MAJOR(sb->s_dev), MINOR(sb->s_dev));
^^^^^
If we enable nfs debug prints using command:
$ rpcdebug -m nfs -s all
write to a file:
$ dd if=/dev/zero of=<NFS Mount>/testfile.txt bs=32768 count=1
Without Patch:
[ 2431.032000] NFS: 0 initiated write call (req 0:12/40, 32768 bytes
@ offset 0) ^^^^
With Patch:
[ 2431.032000] NFS: 0 initiated write call (req 0:18/40, 32768 bytes
@ offset 0) ^^^^
We should store NFS "s->s_id" in decimal to avoid confusion between NFS
flush thread name(in ps output) and NFS debug prints.
Signed-off-by: Vivek Trivedi <[email protected]>
Signed-off-by: Namjae Jeon <[email protected]>
---
fs/nfs/super.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/fs/nfs/super.c b/fs/nfs/super.c
index 3dfa4f1..d64bc3c 100644
--- a/fs/nfs/super.c
+++ b/fs/nfs/super.c
@@ -2047,7 +2047,7 @@ static inline void nfs_initialise_sb(struct super_block *sb)
/* We probably want something more informative here */
snprintf(sb->s_id, sizeof(sb->s_id),
- "%x:%x", MAJOR(sb->s_dev), MINOR(sb->s_dev));
+ "%u:%u", MAJOR(sb->s_dev), MINOR(sb->s_dev));
if (sb->s_blocksize == 0)
sb->s_blocksize = nfs_block_bits(server->wsize,
--
1.7.8.4
Trond,
I agree that s_id is fs specific.
If a NFS client is connected to multiple NFS servers and we want to
analyse NFS IO using rpcdebug prints, ps and top ..
If we store s_id in decimal, it will be a little easier to relate NFS
debug prints with *each* NFS flusher thread activity.
AFIK, In NFS, s_id is mainly used in debug prints, so storing s_id in
decimal may increase readability in NFS debug prints.
Is there any harm if we store NFS s_id in decimal ?
Please let me know your opinion.
Thanks,
Vivek
On Fri, Mar 16, 2012 at 6:29 AM, Myklebust, Trond
<[email protected]> wrote:
> On Thu, 2012-03-15 at 23:58 +0530, Vivek Trivedi wrote:
>> NFS bdi flush thread in ps output is printed like "flush-<major number
>> in decimal>:<minor number in decimal>"
>> For example:
>> $ ps aux | grep flush
>> ?2079 root ? ? ? ? 0 SW ? [flush-0:18]
>> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?^^^^
>>
>> nfs_bdi_register()
>> ==> bdi_register_dev()
>> ==> bdi_register(bdi, NULL, "%u:%u", MAJOR(dev), MINOR(dev));
>> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?^^^^^
>>
>> However, NFS sb->s_id store major:minor number in hex:
>>
>> nfs_initialise_sb()
>> ==> ? ? ? ? snprintf(sb->s_id, sizeof(sb->s_id),
>> ? ? ? ? ? ? ? ? ?"%x:%x", MAJOR(sb->s_dev), MINOR(sb->s_dev));
>> ? ? ? ? ? ? ? ? ? ^^^^^
>>
>> If we enable nfs debug prints using command:
>> $ rpcdebug -m nfs -s all
>>
>> write to a file:
>> $ dd if=/dev/zero of=<NFS Mount>/testfile.txt bs=32768 count=1
>>
>> Without Patch:
>> [ 2431.032000] NFS: ? ? 0 initiated write call (req 0:12/40, 32768 bytes
>> @ offset 0) ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ^^^^
>>
>> With Patch:
>> [ 2431.032000] NFS: ? ? 0 initiated write call (req 0:18/40, 32768 bytes
>> @ offset 0) ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ^^^^
>>
>> We should store NFS "s->s_id" in decimal to avoid confusion between NFS
>> flush thread name(in ps output) and NFS debug prints.
>
> Why do we care? The s_id is entirely filesystem-specific so it shouldn't
> be compared to the properties of VFS objects anyway.
>
> --
> Trond Myklebust
> Linux NFS client maintainer
>
> NetApp
> [email protected]
> http://www.netapp.com
>
T24gVGh1LCAyMDEyLTAzLTE1IGF0IDIzOjU4ICswNTMwLCBWaXZlayBUcml2ZWRpIHdyb3RlOg0K
PiBORlMgYmRpIGZsdXNoIHRocmVhZCBpbiBwcyBvdXRwdXQgaXMgcHJpbnRlZCBsaWtlICJmbHVz
aC08bWFqb3IgbnVtYmVyDQo+IGluIGRlY2ltYWw+OjxtaW5vciBudW1iZXIgaW4gZGVjaW1hbD4i
DQo+IEZvciBleGFtcGxlOg0KPiAkIHBzIGF1eCB8IGdyZXAgZmx1c2gNCj4gIDIwNzkgcm9vdCAg
ICAgICAgIDAgU1cgICBbZmx1c2gtMDoxOF0NCj4gICAgICAgICAgICAgICAgICAgICAgICAgICAg
ICAgICAgXl5eXg0KPiANCj4gbmZzX2JkaV9yZWdpc3RlcigpDQo+ID09PiBiZGlfcmVnaXN0ZXJf
ZGV2KCkNCj4gPT0+IGJkaV9yZWdpc3RlcihiZGksIE5VTEwsICIldToldSIsIE1BSk9SKGRldiks
IE1JTk9SKGRldikpOw0KPiAgICAgICAgICAgICAgICAgICAgICAgICAgICAgIF5eXl5eDQo+IA0K
PiBIb3dldmVyLCBORlMgc2ItPnNfaWQgc3RvcmUgbWFqb3I6bWlub3IgbnVtYmVyIGluIGhleDoN
Cj4gDQo+IG5mc19pbml0aWFsaXNlX3NiKCkNCj4gPT0+ICAgICAgICAgc25wcmludGYoc2ItPnNf
aWQsIHNpemVvZihzYi0+c19pZCksDQo+ICAgICAgICAgICAgICAgICAgIiV4OiV4IiwgTUFKT1Io
c2ItPnNfZGV2KSwgTUlOT1Ioc2ItPnNfZGV2KSk7DQo+ICAgICAgICAgICAgICAgICAgIF5eXl5e
DQo+IA0KPiBJZiB3ZSBlbmFibGUgbmZzIGRlYnVnIHByaW50cyB1c2luZyBjb21tYW5kOg0KPiAk
IHJwY2RlYnVnIC1tIG5mcyAtcyBhbGwNCj4gDQo+IHdyaXRlIHRvIGEgZmlsZToNCj4gJCBkZCBp
Zj0vZGV2L3plcm8gb2Y9PE5GUyBNb3VudD4vdGVzdGZpbGUudHh0IGJzPTMyNzY4IGNvdW50PTEN
Cj4gDQo+IFdpdGhvdXQgUGF0Y2g6DQo+IFsgMjQzMS4wMzIwMDBdIE5GUzogICAgIDAgaW5pdGlh
dGVkIHdyaXRlIGNhbGwgKHJlcSAwOjEyLzQwLCAzMjc2OCBieXRlcw0KPiBAIG9mZnNldCAwKSAg
ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgXl5eXg0KPiANCj4gV2l0aCBQ
YXRjaDoNCj4gWyAyNDMxLjAzMjAwMF0gTkZTOiAgICAgMCBpbml0aWF0ZWQgd3JpdGUgY2FsbCAo
cmVxIDA6MTgvNDAsIDMyNzY4IGJ5dGVzDQo+IEAgb2Zmc2V0IDApICAgICAgICAgICAgICAgICAg
ICAgICAgICAgICAgICAgICAgICAgICBeXl5eDQo+IA0KPiBXZSBzaG91bGQgc3RvcmUgTkZTICJz
LT5zX2lkIiBpbiBkZWNpbWFsIHRvIGF2b2lkIGNvbmZ1c2lvbiBiZXR3ZWVuIE5GUw0KPiBmbHVz
aCB0aHJlYWQgbmFtZShpbiBwcyBvdXRwdXQpIGFuZCBORlMgZGVidWcgcHJpbnRzLg0KDQpXaHkg
ZG8gd2UgY2FyZT8gVGhlIHNfaWQgaXMgZW50aXJlbHkgZmlsZXN5c3RlbS1zcGVjaWZpYyBzbyBp
dCBzaG91bGRuJ3QNCmJlIGNvbXBhcmVkIHRvIHRoZSBwcm9wZXJ0aWVzIG9mIFZGUyBvYmplY3Rz
IGFueXdheS4NCg0KLS0gDQpUcm9uZCBNeWtsZWJ1c3QNCkxpbnV4IE5GUyBjbGllbnQgbWFpbnRh
aW5lcg0KDQpOZXRBcHANClRyb25kLk15a2xlYnVzdEBuZXRhcHAuY29tDQp3d3cubmV0YXBwLmNv
bQ0KDQo=