Re: [PATCH 2/4] staging/lustre/obdclass: read jobid from proc

[Peng Tao]

On Wed, Oct 30, 2013 at 9:21 PM, Greg Kroah-Hartman
<gregkh@xxxxxxxxxxxxxxxxxxx> wrote:
> On Wed, Oct 30, 2013 at 07:30:34PM +0800, Peng Tao wrote:
>> so that we can get rid of cfs_get_environ() that needs access_process_vm() that
>> is a core mm function and is not available on some architectures.
>>
>> Reviewed-by: Niu Yawei <yawei.niu@xxxxxxxxx>
>> Signed-off-by: Peng Tao <bergwolf@xxxxxxxxx>
>> Signed-off-by: Andreas Dilger <andreas.dilger@xxxxxxxxx>
>> ---
>>  drivers/staging/lustre/lustre/obdclass/class_obd.c |  119 +++++++++++++++++++-
>>  1 file changed, 115 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/staging/lustre/lustre/obdclass/class_obd.c b/drivers/staging/lustre/lustre/obdclass/class_obd.c
>> index b1024a6..99ce863 100644
>> --- a/drivers/staging/lustre/lustre/obdclass/class_obd.c
>> +++ b/drivers/staging/lustre/lustre/obdclass/class_obd.c
>> @@ -96,8 +96,117 @@ EXPORT_SYMBOL(obd_dirty_transit_pages);
>>  char obd_jobid_var[JOBSTATS_JOBID_VAR_MAX_LEN + 1] = JOBSTATS_DISABLE;
>>  EXPORT_SYMBOL(obd_jobid_var);
>>
>> -/* Get jobid of current process by reading the environment variable
>> - * stored in between the "env_start" & "env_end" of task struct.
>> +static char *self_environ_file = "/proc/self/environ";
>
> Heh, no, that's not ok at all.
>
> This is a _huge_ sign that you are doing something wrong in your driver
> if you need something that isn't exported, or that you have to dig out
> of proc.
>
Lustre has a functionality that lets applications specify a global
jobid by setting the same environment variable across the cluster and
thus server can trace IO that is issued by different job. Currently it
is implemented by having a private version of cfs_access_process_vm
because access_process_vm is not exported. The patch intends to change
it to reading from proc. If we cannot dig it out from proc either, how
can we keep the functionality then? Please advise.

Thanks,
Tao

> Sorry, I can't take this, please fix the underlying problems that would
> even think that you need access to the environment from within a kernel
> driver.
>
> greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/