2005-09-23 12:37:56

by [email protected]

[permalink] [raw]
Subject: max_fd

Hi

What happend to files_struct.max_fd? Is it safe to use
files_fdtable(files_struct).max_fds?

thanks,

Pablo Fernandez


2005-09-23 16:01:14

by Dipankar Sarma

[permalink] [raw]
Subject: Re: max_fd

On Fri, Sep 23, 2005 at 02:37:51PM +0200, Pablo Fernandez wrote:
> Hi
>
> What happend to files_struct.max_fd? Is it safe to use
> files_fdtable(files_struct).max_fds?

No.

Just do -

struct fdtable *fdt;

rcu_read_lock();
fdt = files_fdtable(files_struct);
if (fdt->max_fds......
...
rcu_read_unlock();


If you are updating the fd table, then acuire the file_lock
instead of rcu_read_lock/unlock.

Thanks
Dipankar

2005-09-23 17:03:47

by Frank van Maarseveen

[permalink] [raw]
Subject: Re: max_fd

On Fri, Sep 23, 2005 at 09:25:10PM +0530, Dipankar Sarma wrote:
> On Fri, Sep 23, 2005 at 02:37:51PM +0200, Pablo Fernandez wrote:
> > Hi
> >
> > What happend to files_struct.max_fd? Is it safe to use
> > files_fdtable(files_struct).max_fds?
>
> No.
>
> Just do -
>
> struct fdtable *fdt;
>
> rcu_read_lock();
> fdt = files_fdtable(files_struct);
> if (fdt->max_fds......
> ...
> rcu_read_unlock();

In include/linux/file.h I see this:

#define files_fdtable(files) (rcu_dereference((files)->fdt))

looks better to me unless you really want to update the
struct fdtable.

--
Frank

2005-09-23 17:32:54

by Dipankar Sarma

[permalink] [raw]
Subject: Re: max_fd

On Fri, Sep 23, 2005 at 07:03:45PM +0200, Frank van Maarseveen wrote:
> On Fri, Sep 23, 2005 at 09:25:10PM +0530, Dipankar Sarma wrote:
> > On Fri, Sep 23, 2005 at 02:37:51PM +0200, Pablo Fernandez wrote:
> > Just do -
> >
> > struct fdtable *fdt;
> >
> > rcu_read_lock();
> > fdt = files_fdtable(files_struct);
> > if (fdt->max_fds......
> > ...
> > rcu_read_unlock();
>
> In include/linux/file.h I see this:
>
> #define files_fdtable(files) (rcu_dereference((files)->fdt))
>
> looks better to me unless you really want to update the
> struct fdtable.

I would much rather have it my way :)

Well, the main reason is that if that code is somehow copied
by to a lock-free critical section, it could cause problems.
If you dereference ->fdt multiple times in a lock-free
section, you could see two different pointers due to
a concurrent update.

I would advise sticking to the same convention everywhere.

Thanks
Dipankar

2005-09-23 18:40:58

by Frank van Maarseveen

[permalink] [raw]
Subject: Re: max_fd

On Fri, Sep 23, 2005 at 10:56:53PM +0530, Dipankar Sarma wrote:
>
> Well, the main reason is that if that code is somehow copied
> by to a lock-free critical section, it could cause problems.
> If you dereference ->fdt multiple times in a lock-free
> section, you could see two different pointers due to
> a concurrent update.

thanks for the explanation. This raises a lot of other questions. What
if max_fds is updated by RCU right after obtaining it...

--
Frank

2005-09-23 18:52:13

by Dipankar Sarma

[permalink] [raw]
Subject: Re: max_fd

On Fri, Sep 23, 2005 at 08:40:56PM +0200, Frank van Maarseveen wrote:
> On Fri, Sep 23, 2005 at 10:56:53PM +0530, Dipankar Sarma wrote:
> >
> > Well, the main reason is that if that code is somehow copied
> > by to a lock-free critical section, it could cause problems.
> > If you dereference ->fdt multiple times in a lock-free
> > section, you could see two different pointers due to
> > a concurrent update.
>
> thanks for the explanation. This raises a lot of other questions. What
> if max_fds is updated by RCU right after obtaining it...

If you are updating it, you hold the lock. That way you can't
be racing with another update. Secondly, changing max_fds
would require allocating a new fdtable structure and
updating ->fdt to point to the new structure, all under
the files_struct lock. The lock-free readers may see the
older fdtable which is kept around until the RCU grace
period is over. That makes it safe.

Thanks
Dipankar