From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933980AbbLVWEI (ORCPT ); Tue, 22 Dec 2015 17:04:08 -0500 Received: from mga14.intel.com ([192.55.52.115]:47671 "EHLO mga14.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S933965AbbLVWEE convert rfc822-to-8bit (ORCPT ); Tue, 22 Dec 2015 17:04:04 -0500 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.20,466,1444719600"; d="scan'208";a="713103362" From: "Dilger, Andreas" To: Niranjan Dighe CC: "Drokin, Oleg" , "Eremin, Dmitry" , James Simmons , "Mike Rapoport" , Patrick Boettcher , Matthew Tyler , "devel@driverdev.osuosl.org" , "linux-kernel@vger.kernel.org" , "lustre-devel@lists.lustre.org" , "Greg Kroah-Hartman" Subject: Re: [PATCH] staging: lustre/lustre/libcfs: Fix type mismatch reported by sparse Thread-Topic: [PATCH] staging: lustre/lustre/libcfs: Fix type mismatch reported by sparse Thread-Index: AQHRMqREzHvyB9sNM0S+VsIqbmTLAp7Ws/WAgADfsoCAACEoAA== Date: Tue, 22 Dec 2015 22:04:03 +0000 Message-ID: References: <20151209170813.GA12216@codebox> <20151221234443.GA11245@kroah.com> In-Reply-To: Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.254.42.130] Content-Type: text/plain; charset="us-ascii" Content-ID: Content-Transfer-Encoding: 8BIT MIME-Version: 1.0 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 2015/12/22, 06:05, "Niranjan Dighe" wrote: >On Tue, Dec 22, 2015 at 5:14 AM, Greg Kroah-Hartman > wrote: >> On Wed, Dec 09, 2015 at 10:38:13PM +0530, Niranjan Dighe wrote: >>> The third argument to function kportal_memhog_alloc is expected to >>> be gfp_t whereas the actual argument was unsigned int. Fix this by >>> explicitly typecasting to gfp_t >>> >>> Signed-off-by: Niranjan Dighe >>> --- >>> drivers/staging/lustre/lustre/libcfs/module.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/drivers/staging/lustre/lustre/libcfs/module.c >>>b/drivers/staging/lustre/lustre/libcfs/module.c >>> index 96d9d46..9c79f6e 100644 >>> --- a/drivers/staging/lustre/lustre/libcfs/module.c >>> +++ b/drivers/staging/lustre/lustre/libcfs/module.c >>> @@ -268,7 +268,7 @@ static int libcfs_ioctl_int(struct cfs_psdev_file >>>*pfile, unsigned long cmd, >>> /* XXX The ioc_flags is not GFP flags now, need >>>to be fixed */ >>> err = kportal_memhog_alloc(pfile->private_data, >>> data->ioc_count, >>> - data->ioc_flags); >>> + (__force gfp_t)data->ioc_flags); >> >> No, please fix the type to be correct properly, like the comment says >> needs to be done. >> >> thanks, >> >> greg k-h > >Hello Greg, > >I could see that the ioc_flags member of the struct libcfs_ioctl_data >is used as gfp_t only in the >case of the ioctl IOC_LIBCFS_MEMHOG. I can think of following ways to >correct it - > >1. Create a union that has 2 different types encapsulated, something like >this - > union { > __u32 ioc_flags; > gfp_t alloc_flags; > }flags; >Because, the ioc_flags seems to be used in different contexts at >different places throughout the >drivers/staging/lustre directory. > >2. Is it OK to hardcode the appropriate gfp_t flags for the >IOC_LIBCFS_MEMHOG, as the userspace >seems to be taking the decision about the page allocation >zone/strategy, is this what is intended? The memhog functionality is used to introduce memory pressure on a client or server during operation to test error handling as well as memory allocation deadlocks (e.g. GFP_KERNEL used where GFP_NOFS should be used). There are other ways to do this in the kernel today, so all of the memhog code could just be deleted I think. This looks like kportal_memhog_alloc(), kportal_memhog_free(), IOC_LIBCFS_MEMHOG, and struct libcfs_device_userstate could be removed. Cheers, Andreas