From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-yb0-f195.google.com ([209.85.213.195]:35556 "EHLO mail-yb0-f195.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753507AbeFJRtR (ORCPT ); Sun, 10 Jun 2018 13:49:17 -0400 Received: by mail-yb0-f195.google.com with SMTP id f79-v6so5983937ybg.2 for ; Sun, 10 Jun 2018 10:49:17 -0700 (PDT) MIME-Version: 1.0 In-Reply-To: References: <20180607150217.jq757ncqopuimbkd@quack2.suse.cz> <20180608132737.etbkpqg77yz3vhp7@quack2.suse.cz> From: Amir Goldstein Date: Sun, 10 Jun 2018 20:49:16 +0300 Message-ID: Subject: Re: [GIT PULL] Fsnotify cleanups To: Jan Kara Cc: linux-fsdevel , Linus Torvalds Content-Type: text/plain; charset="UTF-8" Sender: linux-fsdevel-owner@vger.kernel.org List-ID: On Sat, Jun 9, 2018 at 9:46 PM, Amir Goldstein wrote: > On Sat, Jun 9, 2018 at 8:30 PM, Linus Torvalds > wrote: >> On Fri, Jun 8, 2018 at 11:57 PM Amir Goldstein wrote: >>> >>> We embed fsnotify_obj in struct inode and fsnotify_obj_mask in struct mount. >> >> So I'd *really* like to see just a pointer, not an embedded struct. >> >> Yes, if you get rid of the mask from the embedded struct (so that it >> only contains a pointer), you do get rid of the odd alignment issues >> and the need for "packed". >> >> But from previous experience, once you embed a structure, that >> structure tends to grow. Because it can, and it's so convenient. >> Suddently it has a spinlock in it too etc. >> > > Fair enough. > >> So if you can make do with just the pointer, it would actually be >> nicer to expose it as such. Then you can also avoid the header file >> dependency chain, because you can just pre-declare the structure (like >> it does now) and >> >> struct fsnotify_mark_connector; >> .. >> struct fsnotify_mark_connector __rcu *i_fsnotify_marks; >> >> in the inode. That way the core header files don't need to worry about >> the fsnotify details, and don't need to include fsnotify headers. >> >> And we can do inode packing without having to know (not that it >> happens all that often - everybody would *love* to shrink the inode >> structure, but it's just hard. Because everybody also wants to put >> their own data into the inode ..) >> >> Can't the generalization code just take a pointer to a __rcu pointer >> to fsnotify_mark_connector, obviating the need for the fsnotify_obj >> structure definition? >> > Jan, I reworked the cleanup patches to get rid of fsnotify_obj and pushed to: https://github.com/amir73il/linux.git fsnotify-cleanup Only last 5 patches from fsnotify_for_v4.18-rc1 have been modified and I removed your S-O-B from the modified patches. This leaves struct inode unchanged, in fact no changes to code outside fsnotify/audit at all. mask is now a member of connector for the purpose of generalizing add/remove mark, but struct inode/mount still have a copy of the mask for the purpose of the VFS optimizations. I have a POC patch that removes mask from struct inode and uses 2 bits in connector pointer for VFS optimizations, but that patch requires more testing, so for another time. Thanks, Amir.