Details
-
New Feature
-
Resolution: Fixed
-
Minor
-
None
-
None
-
9223372036854775807
Description
Limit the rate of ptlrpc requests by uid or gid. By this way, the TBF policy is able to provide QoS control to a user or user group identified by uid or gid.
.
Attachments
Issue Links
Activity
Changing the protocol means that this feature can likely not land until 2.12, and will not be easily backported to 2.10 or earlier releases.
I agree that re-using the existing uid/gid fields does add some complexity, but allows this feature to work with existing clients and servers much more easily.
Why not initialize rq_pill in all cases to avoid this problem?
Hi, Peter, in my former patches, I was trying to unpack the uid and gid from ptlrpc request to avoid change the protocol. When I look into the bugs that Mahmoud Hanafi mentioned, I noticed that, ptlrpc_request::rq_pill is initialized in ptlrpc_service_ops::so_hpreq_handler. However not every request use so_hpreq_handler.
mdt/mdt_mds.c: .so_hpreq_handler = ptlrpc_hpreq_handler,
mdt/mdt_mds.c: .so_hpreq_handler = NULL,
mdt/mdt_mds.c: .so_hpreq_handler = NULL,
mdt/mdt_mds.c: .so_hpreq_handler = NULL,
mdt/mdt_mds.c: .so_hpreq_handler = NULL,
mdt/mdt_mds.c: .so_hpreq_handler = NULL,ost/ost_handler.c: .so_hpreq_handler = ptlrpc_hpreq_handler,
ost/ost_handler.c: .so_hpreq_handler = tgt_hpreq_handler,
ost/ost_handler.c: .so_hpreq_handler = NULL,
ost/ost_handler.c: .so_hpreq_handler = NULL,
That means, not every ptlrpc_request:rq_pill has been initialized before it is used in nrs_tbf.c. When the rq_pill is not initialized, using req_capsule_client_get() will trigger a LBUG. Ptlrpc requests from client to MDS, belongs to this case, So it caused a LBUG.
I think adding the uid and gid to ptlrpc protocol can easily address this issue and it is a better way to keep the logic simpler and easier to understand. Although it will introduce some duplication field in the ptlrpc request, and break the protocol, I think it is worth doing so.
. Our idea is as follows:
1. Add uid and gid to struct ptlrpc_body_v3 similar as jobid.
2. Combine the process logic of uid, gid, jobid together.
3. Handle uid and gid in nrs_tbf similar as jobid.
I uploaded the patch at https://review.whamcloud.com/#/c/27608/. And it need further improvement in coding to better merge the codes of jobid and uid/gid.
We have started to test this patch at nasa.
setting up simple case
lctl set_param mds.MDS.mdt.nrs_policies="tbf uid" lctl set_param mds.MDS.mdt.nrs_tbf_rule="start ext_ug uid=\{11312} rate=100" nbp7-mds2 /proc/fs/lustre/mds/MDS/mdt # cat nrs_tbf_rule regular_requests: CPT 0: ext_ug \{11312} 100, ref 0 default \{*} 10000, ref 0 CPT 1: ext_ug \{11312} 100, ref 0 default \{*} 10000, ref 0 high_priority_requests: CPT 0: ext_ug \{11312} 100, ref 0 default \{*} 10000, ref 1 CPT 1: ext_ug \{11312} 100, ref 0 default \{*} 10000, ref 1
Just cd into mount directory get a crash
[ 456.579558] BUG: unable to handle kernel NULL pointer dereference at 0000000000000058 [ 456.588322] IP: [<ffffffffa0f83132>] nrs_tbf_id_cli_set+0x52/0xa0 [ptlrpc] [ 456.596114] PGD 811980067 PUD 800580067 PMD 0 [ 456.601103] Oops: 0000 [#1] SMP Stack traceback for pid 3081 0xffff880762acbf40 3081 2 1 3 R 0xffff880762acc5d8 *mdt01_000 ffff8800a72cfcb8 0000000000000018 ffffffffa0f83100 ffff8810124a1400 ffff880f7fbfc100 ffff880f7fbfc100 ffff8800a72cfd00 ffffffffa0f831f7 00000010a72cfdb0 0000000000000000 00000000fcdbf389 ffff8810124a1400 Call Trace: [<ffffffffa0f83100>] ? nrs_tbf_id_cli_set+0x20/0xa0 [ptlrpc] [<ffffffffa0f831f7>] ? nrs_tbf_id_cli_find+0x37/0xa0 [ptlrpc] [<ffffffffa0f85f83>] ? nrs_tbf_res_get+0x43/0x4c0 [ptlrpc] [<ffffffffa0f7839c>] ? nrs_resource_get+0x7c/0x100 [ptlrpc] [<ffffffffa0f78910>] ? nrs_resource_get_safe+0x80/0xf0 [ptlrpc] [<ffffffffa0f7c3e3>] ? ptlrpc_nrs_req_initialize+0x83/0x100 [ptlrpc] [<ffffffffa0f4b211>] ? ptlrpc_main+0x1771/0x1e40 [ptlrpc] [<ffffffff81029557>] ? __switch_to+0xd7/0x4c0 [<ffffffff81689b00>] ? __schedule+0x2e0/0x8a0 [<ffffffffa0f49aa0>] ? ptlrpc_register_service+0xe30/0xe30 [ptlrpc] [<ffffffff810ad9ef>] ? kthread+0xcf/0xe0 [<ffffffff810ad920>] ? insert_kthread_work+0x40/0x40 [<ffffffff81695ad8>] ? ret_from_fork+0x58/0x90 [<ffffffff810ad920>] ? insert_kthread_work+0x40/0x40 crash> bt -t PID: 13793 TASK: ffff8807fb598fd0 CPU: 9 COMMAND: "mdt01_001" START: machine_kexec at ffffffff8105a3ab [ffff880780ddba18] machine_kexec at ffffffff8105a3ab [ffff880780ddba78] __crash_kexec at ffffffff811015b2 [ffff880780ddbb00] __crash_kexec at ffffffff8110164f [ffff880780ddbb48] panic at ffffffff8167f10d [ffff880780ddbbc8] lbug_with_loc at ffffffffa0913854 [libcfs] [ffff880780ddbbe8] __req_capsule_get at ffffffffa0c8ecc8 [ptlrpc] [ffff880780ddbc68] req_capsule_client_get at ffffffffa0c8ed75 [ptlrpc] [ffff880780ddbc78] unpack_ugid_from_mdt_body at ffffffffa0cb0f60 [ptlrpc] [ffff880780ddbc90] mdt_tbf_id_cli_set at ffffffffa0cb10bb [ptlrpc] [ffff880780ddbcb0] nrs_tbf_id_cli_set at ffffffffa0cb1163 [ptlrpc] [ffff880780ddbcd8] nrs_tbf_id_cli_find at ffffffffa0cb11f7 [ptlrpc] [ffff880780ddbd08] nrs_tbf_res_get at ffffffffa0cb3f83 [ptlrpc] [ffff880780ddbd50] nrs_resource_get at ffffffffa0ca639c [ptlrpc] [ffff880780ddbd98] nrs_resource_get_safe at ffffffffa0ca6910 [ptlrpc] [ffff880780ddbdd0] ptlrpc_nrs_req_initialize at ffffffffa0caa3e3 [ptlrpc] [ffff880780ddbde8] ptlrpc_main at ffffffffa0c79211 [ptlrpc] [ffff880780ddbdf8] __switch_to at ffffffff81029557 [ffff880780ddbe58] __schedule at ffffffff81689b00 [ffff880780ddbea8] ptlrpc_main at ffffffffa0c77aa0 [ptlrpc] [ffff880780ddbec8] kthread at ffffffff810ad9ef [ffff880780ddbf30] kthread at ffffffff810ad920 [ffff880780ddbf50] ret_from_fork at ffffffff81695ad8 [ffff880780ddbf80] kthread at ffffffff810ad920
Anonymous Coward (jjkky@yahoo.com) uploaded a new patch: https://review.whamcloud.com/27608
Subject: LU-9658 ptlrpc: Add QoS for uid and gid in NRS-TBF
Project: fs/lustre-release
Branch: master
Current Patch Set: 1
Commit: f7c2c29d69ec22095c721888bd54b7b860d5c19f
Hi, Andreas. I analyzed the ptlrpc codes and its processing logic on server side(both mdt and ost) these days. Initializing rq_pill in requests is a feasible solution, although doing this is tiring. I have done the related codes without breaking the original ptlrpc handling logic, and I have uploaded the patch.
In TBF, I think, we do not need limit every request. Only limiting the requests that have great influence on performance is enough (such as read, write, getattr, setattr etc). For others, (such connect, statfs, quotactl, ping), we might just let them go. Besides, if we need to control every request from client, I think, modifying the request protocol is inevitable and is also favorable in the long run.