lkml.org 
[lkml]   [2010]   [Jan]   [1]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] sysfs: Cache the last sysfs_dirent to improve readdir scalability v2


On Fri, 1 Jan 2010, Eric W. Biederman wrote:
> mutex_unlock(&sysfs_mutex);
> + ret = filldir(dirent, name, len, filp->f_pos, ino, type);
> + mutex_lock(&sysfs_mutex);
> + if (ret < 0)
> + break;
> + }
> + mutex_unlock(&sysfs_mutex);
> + if ((filp->f_pos > 1) && !pos) { /* EOF */
> + filp->f_pos = INT_MAX;
> + filp->private_data = NULL;
> }
> return 0;

That

mutex_lock(&sysfs_mutex);
if (ret < 0)
break;

looks just silly. We know 'pos' is non-NULL, so the break will effectively
just be a "mutex_unlock + return 0", and we just did the mutex_lock, so
why not instead do

if (ret < 0)
return 0;
mutex_lock(&sysfs_mutex);

there?

Not that it really _matters_, but it seems way clearer, no?

But other than that mindless nit, I can't see anything wrong with your
logic, and it looks ok to me from just reading the patch itself.

So I guess that's an "Ack", although I'd prefer it to get some more
testing and perhaps go through Greg's tree as sysfs patches usually go.

And by "testing" I mean both the "yes, this second version also breaks the
lockdep chain and avoids the warning", but also some kind of actual
testing of /sysfs itself. If there is any.

Linus


\
 
 \ /
  Last update: 2010-01-02 00:15    [W:0.232 / U:0.036 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site