Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754465Ab3J3Nbr (ORCPT ); Wed, 30 Oct 2013 09:31:47 -0400 Received: from mail-we0-f173.google.com ([74.125.82.173]:50688 "EHLO mail-we0-f173.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751230Ab3J3Nbq (ORCPT ); Wed, 30 Oct 2013 09:31:46 -0400 MIME-Version: 1.0 In-Reply-To: <20131030132101.GD30087@kroah.com> References: <1383132636-8952-1-git-send-email-bergwolf@gmail.com> <1383132636-8952-2-git-send-email-bergwolf@gmail.com> <20131030132101.GD30087@kroah.com> From: Peng Tao Date: Wed, 30 Oct 2013 21:31:24 +0800 Message-ID: Subject: Re: [PATCH 2/4] staging/lustre/obdclass: read jobid from proc To: Greg Kroah-Hartman Cc: Linux Kernel Mailing List , Andreas Dilger Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2350 Lines: 52 On Wed, Oct 30, 2013 at 9:21 PM, Greg Kroah-Hartman 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 >> Signed-off-by: Peng Tao >> Signed-off-by: Andreas Dilger >> --- >> 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@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/