public inbox for netdev@vger.kernel.org 
 help / color / mirror / Atom feed
From: ebiederm@xmission•com (Eric W. Biederman)
To: Tejun Heo <tj@kernel•org>
Cc: Greg Kroah-Hartman <gregkh@suse•de>,
	Kay Sievers <kay.sievers@vrfy•org>,
	linux-kernel@vger•kernel.org,
	Cornelia Huck <cornelia.huck@de•ibm.com>,
	linux-fsdevel@vger•kernel.org,
	Eric Dumazet <eric.dumazet@gmail•com>,
	Benjamin LaHaise <bcrl@lhnet•ca>, Serge Hallyn <serue@us•ibm.com>,
	netdev@vger•kernel.org
Subject: Re: [PATCH 1/6] sysfs: Basic support for multiple super blocks
Date: Tue, 30 Mar 2010 22:51:35 -0700	[thread overview]
Message-ID: <m1634d82e0.fsf@fess.ebiederm.org> (raw)
In-Reply-To: <4BB2E098.7030202@kernel.org> (Tejun Heo's message of "Wed\, 31 Mar 2010 14\:41\:44 +0900")

Tejun Heo <tj@kernel•org> writes:

> Hello, Eric.
>
> On 03/31/2010 03:31 AM, Eric W. Biederman wrote:
>> From: Eric W. Biederman <ebiederm@xmission•com>
>> 
>> Add all of the necessary bioler plate to support
>                            boiler :-)
>
>> +static int sysfs_test_super(struct super_block *sb, void *data)
>> +{
>> +	struct sysfs_super_info *sb_info = sysfs_info(sb);
>> +	struct sysfs_super_info *info = data;
>> +	int found = 1;
>> +	return found;
>> +}
>
> Can you please make it return bool?

Nope.  That would mean I could not use it with sget.

>>  static int sysfs_get_sb(struct file_system_type *fs_type,
>>  	int flags, const char *dev_name, void *data, struct vfsmount *mnt)
>>  {
>> -	return get_sb_single(fs_type, flags, data, sysfs_fill_super, mnt);
>> +	struct sysfs_super_info *info;
>> +	struct super_block *sb;
>> +	int error;
>> +
>> +	error = -ENOMEM;
>> +	info = kzalloc(sizeof(*info), GFP_KERNEL);
>> +	if (!info)
>> +		goto out;
>> +	sb = sget(fs_type, sysfs_test_super, sysfs_set_super, info);
>> +	if (IS_ERR(sb) || sb->s_fs_info != info)
>> +		kfree(info);
>> +	if (IS_ERR(sb)) {
>> +		kfree(info);
>> +		error = PTR_ERR(sb);
>> +		goto out;
>> +	}
>> +	if (!sb->s_root) {
>> +		sb->s_flags = flags;
>> +		error = sysfs_fill_super(sb, data, flags & MS_SILENT ? 1 : 0);
>> +		if (error) {
>> +			deactivate_locked_super(sb);
>> +			goto out;
>> +		}
>> +		sb->s_flags |= MS_ACTIVE;
>> +	}
>> +
>> +	simple_set_mnt(mnt, sb);
>> +	error = 0;
>> +out:
>> +	return error;
>> +}
>
> I haven't looked at later patches but I suppose this is gonna be
> filled with more meaningful stuff later. 

Yes it will.

> One (possibly silly) thing
> that stands out compared to get_sb_single() is missing remount
> handling.  Is it intended?

There is nothing for a remount to do so I ignore it.   The only
thing that would possibly be meaningful is a read-only mount,
and nothing I know of sysfs suggests read-only mounts of sysfs
work, or make any sense.

>> index 30f5a44..030a39d 100644
>> --- a/fs/sysfs/sysfs.h
>> +++ b/fs/sysfs/sysfs.h
>> @@ -114,6 +114,9 @@ struct sysfs_addrm_cxt {
>>  /*
>>   * mount.c
>>   */
>> +struct sysfs_super_info {
>> +};
>> +#define sysfs_info(SB) ((struct sysfs_super_info *)(SB->s_fs_info))
>
> Another nit picking.  It would be better to wrap SB in the macro
> definition.  Also, wouldn't an inline function be better?

Good spotting.  That doesn't bite today but it will certainly bite
someday if it isn't fixed.

I wonder how that has slipped through the review all of this time.

Eric

  reply	other threads:[~2010-03-31  5:51 UTC|newest]

Thread overview: 82+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-03-30 18:30 [PATCH 0/6] tagged sysfs support Eric W. Biederman
2010-03-30 18:31 ` [PATCH 1/6] sysfs: Basic support for multiple super blocks Eric W. Biederman
2010-03-30 19:23   ` Eric Dumazet
2010-03-30 23:50     ` [PATCH 7/6] sysfs: Remove double free sysfs_get_sb Eric W. Biederman
2010-03-31  5:01   ` [PATCH 1/6] sysfs: Basic support for multiple super blocks Serge E. Hallyn
2010-03-31  5:01     ` Serge E. Hallyn
2010-03-31  5:41   ` Tejun Heo
2010-03-31  5:51     ` Eric W. Biederman [this message]
2010-03-31 13:47       ` Serge E. Hallyn
2010-03-31 14:02         ` Eric W. Biederman
2010-04-05  7:45       ` Tejun Heo
2010-04-29 20:29   ` patch sysfs-basic-support-for-multiple-super-blocks.patch added to gregkh-2.6 tree gregkh
2010-03-30 18:31 ` [PATCH 2/6] kobj: Add basic infrastructure for dealing with namespaces Eric W. Biederman
2010-04-29 20:29   ` patch kobj-add-basic-infrastructure-for-dealing-with-namespaces.patch added to gregkh-2.6 tree gregkh
2010-03-30 18:31 ` [PATCH 3/6] sysfs: Implement sysfs tagged directory support Eric W. Biederman
2010-03-31  2:43   ` Serge E. Hallyn
2010-03-31  3:38     ` Eric W. Biederman
2010-03-31  4:02       ` Serge E. Hallyn
2010-03-31  4:23         ` Eric W. Biederman
2010-03-31  4:53           ` Serge E. Hallyn
2010-03-31  6:49   ` Tejun Heo
2010-03-31  7:43     ` Eric W. Biederman
2010-03-31  8:17       ` Tejun Heo
2010-03-31  8:22         ` Tejun Heo
2010-03-31  9:39           ` Eric W. Biederman
2010-04-05  8:17             ` Tejun Heo
2010-04-29 20:29   ` patch sysfs-implement-sysfs-tagged-directory-support.patch added to gregkh-2.6 tree gregkh
2010-04-30  4:18     ` Tejun Heo
2010-04-30  4:45       ` Greg KH
2010-04-30  5:24         ` Eric W. Biederman
2010-04-30  5:37           ` Tejun Heo
2010-04-30  6:12             ` Tejun Heo
2010-04-30 14:29             ` Serge E. Hallyn
2010-04-30 15:22               ` Tejun Heo
2010-04-30 15:43                 ` Serge E. Hallyn
2010-04-30 15:58                   ` Greg KH
2010-03-30 18:31 ` [PATCH 4/6] sysfs: Add support for tagged directories with untagged members Eric W. Biederman
2010-04-29 20:29   ` patch sysfs-add-support-for-tagged-directories-with-untagged-members.patch added to gregkh-2.6 tree gregkh
2010-03-30 18:31 ` [PATCH 5/6] sysfs: Implement sysfs_delete_link Eric W. Biederman
2010-04-29 20:29   ` patch sysfs-implement-sysfs_delete_link.patch added to gregkh-2.6 tree gregkh
2010-03-30 18:31 ` [PATCH 6/6] driver core: Implement ns directory support for device classes Eric W. Biederman
2010-04-29 20:29   ` patch driver-core-implement-ns-directory-support-for-device-classes.patch added to gregkh-2.6 tree gregkh
2010-03-30 18:53 ` [PATCH 0/6] tagged sysfs support Kay Sievers
2010-03-30 23:04   ` Eric W. Biederman
2010-03-31  5:51     ` Kay Sievers
2010-03-31  6:25       ` Tejun Heo
2010-03-31  6:52       ` Eric W. Biederman
2010-04-03  0:58       ` Ben Hutchings
2010-04-03  8:35         ` Kay Sievers
2010-04-03 16:05           ` Ben Hutchings
2010-04-03 16:35             ` Kay Sievers
2010-03-31 17:21 ` Serge E. Hallyn
2010-03-31 18:09   ` Eric W. Biederman
2010-05-05  0:35 ` [PATCH 0/6] netns support in the kobject layer Eric W. Biederman
2010-05-06 20:04   ` Greg KH
2010-05-16  6:26     ` David Miller
2010-05-17 18:11       ` Greg KH
2010-05-17 20:58         ` Eric W. Biederman
2010-05-17 21:03           ` Greg KH
2010-05-17 22:37             ` Eric W. Biederman
2010-05-17 22:54               ` Greg KH
2010-05-17 23:48             ` David Miller
2010-05-18  4:08               ` Greg KH
2010-05-18  4:21                 ` David Miller
2010-05-05  0:36 ` [PATCH 1/6] kobject: Send hotplug events in all network namespaces Eric W. Biederman
2010-05-20 18:10   ` patch kobject-send-hotplug-events-in-all-network-namespaces.patch added to gregkh-2.6 tree gregkh
2010-05-05  0:36 ` [PATCH 2/6] netns: Teach network device kobjects which namespace they are in Eric W. Biederman
2010-05-05 15:17   ` Serge E. Hallyn
2010-05-05 19:56     ` Eric W. Biederman
2010-05-05 22:01       ` Serge E. Hallyn
2010-05-17  4:59         ` [PATCH 7/6] net/sysfs: Fix the bitrot in network device kobject namespace support Eric W. Biederman
2010-05-17  5:07           ` David Miller
2010-05-20 18:10   ` patch netns-teach-network-device-kobjects-which-namespace-they-are-in.patch added to gregkh-2.6 tree gregkh
2010-05-05  0:36 ` [PATCH 3/6] netlink: Implment netlink_broadcast_filtered Eric W. Biederman
2010-05-20 18:10   ` patch netlink-implment-netlink_broadcast_filtered.patch added to gregkh-2.6 tree gregkh
2010-05-05  0:36 ` [PATCH 4/6] kobj: Send hotplug events in the proper namespace Eric W. Biederman
2010-05-20 18:10   ` patch kobj-send-hotplug-events-in-the-proper-namespace.patch added to gregkh-2.6 tree gregkh
2010-05-05  0:36 ` [PATCH 5/6] hotplug: netns aware uevent_helper Eric W. Biederman
2010-05-20 18:10   ` patch hotplug-netns-aware-uevent_helper.patch added to gregkh-2.6 tree gregkh
2010-05-05  0:36 ` [PATCH 6/6] net: Expose all network devices in a namespaces in sysfs Eric W. Biederman
2010-05-20 18:10   ` patch net-expose-all-network-devices-in-a-namespaces-in-sysfs.patch added to gregkh-2.6 tree gregkh
2010-05-20 17:47 ` [PATCH 0/6] tagged sysfs support Greg KH

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=m1634d82e0.fsf@fess.ebiederm.org \
    --to=ebiederm@xmission$(echo .)com \
    --cc=bcrl@lhnet$(echo .)ca \
    --cc=cornelia.huck@de$(echo .)ibm.com \
    --cc=eric.dumazet@gmail$(echo .)com \
    --cc=gregkh@suse$(echo .)de \
    --cc=kay.sievers@vrfy$(echo .)org \
    --cc=linux-fsdevel@vger$(echo .)kernel.org \
    --cc=linux-kernel@vger$(echo .)kernel.org \
    --cc=netdev@vger$(echo .)kernel.org \
    --cc=serue@us$(echo .)ibm.com \
    --cc=tj@kernel$(echo .)org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox