From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S964992AbbD0PIv (ORCPT ); Mon, 27 Apr 2015 11:08:51 -0400 Received: from mailout4.w1.samsung.com ([210.118.77.14]:54904 "EHLO mailout4.w1.samsung.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932670AbbD0PIt (ORCPT ); Mon, 27 Apr 2015 11:08:49 -0400 X-AuditID: cbfec7f4-f79c56d0000012ee-fa-553e51005171 Message-id: <553E50EB.3000402@samsung.com> Date: Mon, 27 Apr 2015 17:08:27 +0200 From: Beata Michalska User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130804 Thunderbird/17.0.8 MIME-version: 1.0 To: Greg KH Cc: linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org, linux-api@vger.kernel.org, jack@suse.cz, tytso@mit.edu, adilger.kernel@dilger.ca, hughd@google.com, lczerner@redhat.com, hch@infradead.org, linux-ext4@vger.kernel.org, linux-mm@kvack.org, kyungmin.park@samsung.com, kmpark@infradead.org Subject: Re: [RFC v2 1/4] fs: Add generic file system event notifications References: <1430135504-24334-1-git-send-email-b.michalska@samsung.com> <1430135504-24334-2-git-send-email-b.michalska@samsung.com> <20150427142421.GB21942@kroah.com> In-reply-to: <20150427142421.GB21942@kroah.com> Content-type: text/plain; charset=ISO-8859-1 Content-transfer-encoding: 7bit X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFprKIsWRmVeSWpSXmKPExsVy+t/xy7oMgXahBod+clh8/dLBYnFuwQxG i9MTFjFZPP3Ux2Ixe3ozk8Wty6tYLM42vWG3WPZgM4vF5u8dbBYz591hs9iz9ySLxeVdc9gs 7q35z2rR2vOT3YHPo2VzuceCTaUem1doebx9GOCx6dMkdo+mM0eZPd7vu8rm0bdlFaPHmQVH 2D0+b5IL4IrisklJzcksSy3St0vgyuhtamcqeKtSMfvJTdYGxmUyXYwcHBICJhKnpul1MXIC mWISF+6tZ+ti5OIQEljKKPH/znZmCOcZo8SWvbPZQap4BbQknnyZBWazCKhKvFv1jgnEZhPQ l3g1YyWYLSoQIfHn9D5WiHpBiR+T77GA2CICMhIdS/awgwxlFtjCJDF/+01mkISwgKfE2Q8/ oFavYpR4fXoa2CROoKnNN5ezgdjMAjoS+1unQdnyEpvXvGWewCgwC8mSWUjKZiEpW8DIvIpR NLU0uaA4KT3XUK84Mbe4NC9dLzk/dxMjJLK+7GBcfMzqEKMAB6MSD6/BMttQIdbEsuLK3EOM EhzMSiK8fF52oUK8KYmVValF+fFFpTmpxYcYpTlYlMR55+56HyIkkJ5YkpqdmlqQWgSTZeLg lGpg3PJw28I5L25kOn9dzrWlNHi+eoeq+qy7am03nCTU9X7cV50u/Yz/mUQ6G69xwMUS3xPO J84fePGT+W2bscKab0xnveSnrzl97OV0u1NmuX2vS7Kd1jwuZ+usEJP4bsspf9Ip8PvWA5ul xerbZzh+19PijwhReejC8PPDzkVbt1ZE1u+blJ6YpsRSnJFoqMVcVJwIAA49cT+oAgAA Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 04/27/2015 04:24 PM, Greg KH wrote: > On Mon, Apr 27, 2015 at 01:51:41PM +0200, Beata Michalska wrote: >> Introduce configurable generic interface for file >> system-wide event notifications, to provide file >> systems with a common way of reporting any potential >> issues as they emerge. >> >> The notifications are to be issued through generic >> netlink interface by newly introduced multicast group. >> >> Threshold notifications have been included, allowing >> triggering an event whenever the amount of free space drops >> below a certain level - or levels to be more precise as two >> of them are being supported: the lower and the upper range. >> The notifications work both ways: once the threshold level >> has been reached, an event shall be generated whenever >> the number of available blocks goes up again re-activating >> the threshold. >> >> The interface has been exposed through a vfs. Once mounted, >> it serves as an entry point for the set-up where one can >> register for particular file system events. >> >> Signed-off-by: Beata Michalska >> --- >> Documentation/filesystems/events.txt | 231 ++++++++++ >> fs/Makefile | 1 + >> fs/events/Makefile | 6 + >> fs/events/fs_event.c | 770 ++++++++++++++++++++++++++++++++++ >> fs/events/fs_event.h | 25 ++ >> fs/events/fs_event_netlink.c | 99 +++++ >> fs/namespace.c | 1 + >> include/linux/fs.h | 6 +- >> include/linux/fs_event.h | 58 +++ >> include/uapi/linux/fs_event.h | 54 +++ >> include/uapi/linux/genetlink.h | 1 + >> net/netlink/genetlink.c | 7 +- >> 12 files changed, 1257 insertions(+), 2 deletions(-) >> create mode 100644 Documentation/filesystems/events.txt >> create mode 100644 fs/events/Makefile >> create mode 100644 fs/events/fs_event.c >> create mode 100644 fs/events/fs_event.h >> create mode 100644 fs/events/fs_event_netlink.c >> create mode 100644 include/linux/fs_event.h >> create mode 100644 include/uapi/linux/fs_event.h > > Any reason why you just don't do uevents for the block devices today, > and not create a new type of netlink message and userspace tool required > to read these? The idea here is to have support for filesystems with no backing device as well. Parsing the message with libnl is really simple and requires few lines of code (sample application has been presented in the initial version of this RFC) > >> --- a/fs/Makefile >> +++ b/fs/Makefile >> @@ -126,3 +126,4 @@ obj-y += exofs/ # Multiple modules >> obj-$(CONFIG_CEPH_FS) += ceph/ >> obj-$(CONFIG_PSTORE) += pstore/ >> obj-$(CONFIG_EFIVAR_FS) += efivarfs/ >> +obj-y += events/ > > Always? > >> diff --git a/fs/events/Makefile b/fs/events/Makefile >> new file mode 100644 >> index 0000000..58d1454 >> --- /dev/null >> +++ b/fs/events/Makefile >> @@ -0,0 +1,6 @@ >> +# >> +# Makefile for the Linux Generic File System Event Interface >> +# >> + >> +obj-y := fs_event.o > > Always? Even if the option is not selected? Why is everyone forced to > always use this code? Can't you disable it for the "tiny" systems that > don't need it? > I was considering making it optional and I guess it's worth getting back to this idea. >> +struct fs_trace_entry { >> + atomic_t count; > > Why not just use a 'struct kref' for your count, which will save a bunch > of open-coding of reference counting, and forcing us to audit your code > to verify you got all the corner cases correct? :) > >> + atomic_t active; >> + struct super_block *sb; Not sure if using kref would change much here as the kref would not really make it easier to verify those corner cases, unfortunately. > > Are you properly reference counting this pointer? I didn't see where > that was happening, so I must have missed it. > > thanks, > You haven't. And if I haven't missed anything, the sb is being used only as long as the super is alive. Most of the code operates on sb only if it was explicitly asked to, through call from filesystem. There is also a callback notifying of mount being dropped (which proceeds the call to kill_super) that invalidates the object that depends on it. Still, it should be explicitly stated that the sb is being used through bidding up the s_count counter, though that would require taking the sb_lock. AFAIK, one can get the reference to super block but for a particular device. Maybe it would be worth having it more generic (?). > greg k-h > BR Beata