On Tue, Aug 27, 2019 at 1:58 PM Al Viro <viro@xxxxxxxxxxxxxxxxxx> wrote:
>
> On Tue, Aug 27, 2019 at 10:51:44AM +0200, Miklos Szeredi wrote:
>
> > How about something like this:
> >
> > #if BITS_PER_LONG == 32
> > #define F_COUNT_SHORTTERM ((1UL << 24) + 1)
> > #else
> > #define F_COUNT_SHORTTERM ((1UL << 48) + 1)
> > #endif
> >
> > static inline void get_file_shortterm(struct file *f)
> > {
> > atomic_long_add(F_COUNT_SHORTTERM, &f->f_count);
> > }
> >
> > static inline void put_file_shortterm(struct file *f)
> > {
> > fput_many(f, F_COUNT_SHORTTERM);
> > }
> >
> > static inline bool file_is_last_longterm(struct file *f)
> > {
> > return atomic_long_read(&f->f_count) % F_COUNT_SHORTTERM == 1;
> > }
>
> So 256 threads boinking on the same fdinfo at the same time
> and struct file can be freed right under them?
Nope, 256 threads booking short term refs will result in f_count = 256
(note the +1 in .F_COUNT_SHORTTERM). Which can result in false
negative returned by file_is_last_longterm() but no false freeing.
> Or a bit over
> million of dup(), then forking 15 more children, for that matter...
Can give false positive for file_is_last_longterm() but no false freeing.
255 short term refs + ~16M long term refs together can result in false
freeing, true.
>
> Seriously, it might be OK on 64bit (with something like "no more
> than one reference held by a thread", otherwise you'll run
> into overflows even there - 65536 of your shortterm references
> aren't that much). On 32bit it's a non-starter - too easy to
> overflow.
No, 64bit would be impossible to overflow. But if we have to special
case 32bit then it's not worth it...
Thanks,
Miklos
