svn commit: r368609 - in head/sys: kern sys

[Mateusz Guzik]

Unfortunately inlines mess with __FILE__/__LINE__ by showing the
implementation instead of the consumer. This in particular matters
with https://reviews.freebsd.org/D27600

I failed to find replacements for __ macros which don't suffer the problem.

On 12/13/20, Alexander Richardson <arichardson at freebsd.org> wrote:
> On Sun, 13 Dec 2020 at 18:06, Mateusz Guzik <mjg at freebsd.org> wrote:
>>
>> Author: mjg
>> Date: Sun Dec 13 18:06:24 2020
>> New Revision: 368609
>> URL: https://svnweb.freebsd.org/changeset/base/368609
>>
>> Log:
>>   fd: fix fdrop prediction when closing a fd
>>
>>   Most of the time this is the last reference, contrary to typical fdrop
>> use.
>>
>> Modified:
>>   head/sys/kern/kern_descrip.c
>>   head/sys/sys/file.h
>>
>> Modified: head/sys/kern/kern_descrip.c
>> ==============================================================================
>> --- head/sys/kern/kern_descrip.c        Sun Dec 13 16:26:37 2020
>> (r368608)
>> +++ head/sys/kern/kern_descrip.c        Sun Dec 13 18:06:24 2020
>> (r368609)
>> @@ -2766,7 +2766,7 @@ closef(struct file *fp, struct thread *td)
>>                         FILEDESC_XUNLOCK(fdp);
>>                 }
>>         }
>> -       return (fdrop(fp, td));
>> +       return (fdrop_close(fp, td));
>>  }
>>
>>  /*
>>
>> Modified: head/sys/sys/file.h
>> ==============================================================================
>> --- head/sys/sys/file.h Sun Dec 13 16:26:37 2020        (r368608)
>> +++ head/sys/sys/file.h Sun Dec 13 18:06:24 2020        (r368609)
>> @@ -299,6 +299,17 @@ fhold(struct file *fp)
>>         _error;                                                 \
>>  })
>>
>> +#define        fdrop_close(fp, td)             ({                      \
>> +       struct file *_fp;                                       \
>> +       int _error;                                             \
>> +                                                               \
>> +       _error = 0;                                             \
>> +       _fp = (fp);                                             \
>> +       if (__predict_true(refcount_release(&_fp->f_count)))    \
>> +               _error = _fdrop(_fp, td);                       \
>> +       _error;                                                 \
>> +})
>> +
>>  static __inline fo_rdwr_t      fo_read;
>>  static __inline fo_rdwr_t      fo_write;
>>  static __inline fo_truncate_t  fo_truncate;
>
> Wouldn't this be more readable as a static __inline function (or if
> you are concerned about it not being inlined, __always_inline)? Also
> means you can drop the temporary _fp variable that's there to avoid
> side-effects in macro args.
>
> Regards,
> Alex
>


-- 
Mateusz Guzik <mjguzik gmail.com>