Messages in this thread | | | Date | Tue, 24 Mar 2015 15:01:55 +0100 | From | David Sterba <> | Subject | Re: [PATCH] btrfs: explicitly set control file's private_data |
| |
On Mon, Mar 23, 2015 at 06:34:13PM +0100, Tom Van Braeckel wrote: > The private_data member of the Btrfs control device file > (/dev/btrfs-control) is used to hold the current transaction and needs > to be initialized to NULL to signify that no transaction is in progress. > > We explicitly set the control file's private_data to NULL to be > independent of whatever value the misc subsystem initializes it to. > > Backstory: > ---------- > > The misc subsystem (which is used by /dev/btrfs-control) initializes > a file's private_data to point to the misc device when a driver has > registered a custom open file operation and initializes it to NULL > when a custom open file operation has *not* been provided. > > This subtle quirk is confusing, to the point where kernel code registers > *empty* file open operations to have private_data point to the misc > device structure. > > And it leads to bugs, where the addition or removal of a custom open > file operation surprisingly changes the initial contents of a file's > private_data structure. > > To simplify things in the misc subsystem, a patch [1] has been proposed > to *always* set private_data to point to the misc device instead of > only doing this when a custom open file operation has been registered. > > But before we can fix this in the misc subsystem itself, we need to > modify the (few) drivers that rely on this very subtle behavior. > > [1] https://lkml.org/lkml/2014/12/4/939 > > Signed-off-by: Martin Kepplinger <martink@posteo.de> > Signed-off-by: Tom Van Braeckel <tomvanbraeckel@gmail.com>
Thanks for the explanation.
Acked-by: David Sterba <dsterba@suse.cz>
> +static int btrfs_control_open(struct inode *inode, struct file *file) > +{ > + /* The control file's private_data is used to hold the > + * transaction when it is started and is used to keep > + * track of whether a transaction is already in progress. > + */
That's not the common comment style (newline after /* ) but I'm never sure whether I should nitpick about such things or just say yes to fixes.
> + file->private_data = NULL; > + return 0; > +}
| |