Re: [PATCH v3] ubifs: make ubifs_[get|set]xattr atomic

[Artem Bityutskiy <dedekind1@gmail.com>]

Right now, AFAICS, UBIFS uses the "host" inode mutex to serialize xattr
operations. May be the terminology is not the best, but "host" is the
non-xattr inode (say, a file inode) which "carries" the extended
attributes. So one "host" may have many xattrs.

Now, obviously, locking the entire "host" to change one xattr is very
coarse, because operations on other xattrs get blocked by operations on
other, unrelated xattrs.

So the host inode has fields like "xattr_size" - this needs to be
protected with the host mutex, but operations on individual xattrs may
be protected by xattr mutexes. However, this would be a more complex
locking scheme.


So my point is - use the "host->ui_mutex", not the xattr's mutex,
because this is how it is implemented now.

Re-working and improving locking could be a separate piece of job.

On Fri, 2015-08-07 at 14:07 +0800, Dongsheng Yang wrote:
> This commit make the ubifs_[get|set]xattr protected by ui_mutex.
> 
> Originally, there is a possibility that ubifs_getxattr to get
> a wrong value.
> 
>   P1                                  P2
> ----------                  	----------
> ubifs_getxattr                      ubifs_setxattr
> 					- kfree()
> - memcpy()
> 					- kmemdup()
> 

...

>  
> +	mutex_lock(&ui->ui_mutex);

Or to put it differently, you need to use 'host->ui_mutex', not 'ui
->ui_mutex', because 'ui' is the xattr inode here.