Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1763866AbcLSRCw (ORCPT ); Mon, 19 Dec 2016 12:02:52 -0500 Received: from casper.infradead.org ([85.118.1.10]:50540 "EHLO casper.infradead.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1763811AbcLSRCt (ORCPT ); Mon, 19 Dec 2016 12:02:49 -0500 Date: Mon, 19 Dec 2016 17:02:43 +0000 (GMT) From: James Simmons To: Greg Kroah-Hartman cc: "Dilger, Andreas" , "devel@driverdev.osuosl.org" , James Simmons , Linux Kernel Mailing List , "Drokin, Oleg" , Lustre Development List Subject: Re: [PATCH v2 5/5] staging: lustre: headers: use proper byteorder functions in lustre_idl.h In-Reply-To: <20161213010700.GA21619@kroah.com> Message-ID: References: <1481393161-22623-1-git-send-email-jsimmons@infradead.org> <1481393161-22623-6-git-send-email-jsimmons@infradead.org> <20161210181649.GB8477@kroah.com> <73FD0668-EDE9-4757-98ED-8A15CE5DA17C@intel.com> <20161213010700.GA21619@kroah.com> User-Agent: Alpine 2.20 (LFD 67 2015-01-07) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII X-Spam-Note: CRM114 invocation failed X-Spam-Score: -1.9 (-) X-Spam-Report: SpamAssassin version 3.4.1 on casper.infradead.org summary: Content analysis details: (-1.9 points, 5.0 required) pts rule name description ---- ---------------------- -------------------------------------------------- -0.0 NO_RELAYS Informational: message was not relayed via SMTP -1.9 BAYES_00 BODY: Bayes spam probability is 0 to 1% [score: 0.0000] Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2479 Lines: 46 > On Tue, Dec 13, 2016 at 12:55:01AM +0000, Dilger, Andreas wrote: > > On Dec 12, 2016, at 13:00, James Simmons wrote: > > > > > > > > >> On Sat, Dec 10, 2016 at 01:06:01PM -0500, James Simmons wrote: > > >>> In order for lustre_idl.h to be usable for both user > > >>> land and kernel space it has to use the proper > > >>> byteorder functions. > > >> > > >> Why would userspace need/want all of these inline functions? A uapi > > >> header file should just have a the structures that are passed > > >> user/kernel and any needed ioctls. Why would they ever care about > > >> strange byte flip functions and a ton of inline functions? > > >> > > >> I don't think this is needed, of if it is, I really don't want to see > > >> your crazy userspace code... > > > > > > Sigh. More cleanups were done based on the idea this was okay. The > > > reason this was does was when you look at the headers in > > > include/uapi/linux you see a huge number of headers containing a bunch > > > of inline function. To an outside project looking to merge their work > > > into the kernel they would think this is okay. Hopefully all those > > > broken headers will be cleaned up in the near future. > > > Alright I will look to fixing up our tools to handle this requirement. > > > > These accessor functions are used by both the kernel and userspace > > tools, and keeping them in the lustre_idl.h header avoids duplication > > of code. Similar usage exists in other filesystem related uapi headers > > (e.g. auto_fs4.h, bcache.h, btrfs_tree.h, nilfs2_ondisk.h, swab.h, etc.). > > > > That said, if there is an objection to keeping these macros/inline funcs > > in the uapi headers, they still need to exist in the kernel and should > > be kept in the lustre/include/lustre directory and we'll keep a separate > > copy of the macros for userspace. > > "simple" accessors/setters are fine, but these start to get complex, you > are using unlikely, and debug macros and lots of other fun stuff. Do > all other filesystems also do complex stuff like ostid_to_fid()? So the rejection of the byteorder patch was more due to the state of headers than the patch itself. I do have other patches with the cleanup of debugging macros etc but I was submitting the change one change at a time. I will post what cleanups I was looking to do for lustre_ostid.h and lustre_fid.h UAPI headers. This way you can give feedback on what is okay and what has to change.