Re: [PATCH v5 3/5] fcntl: add F_{SET/GET}_RW_HINT_EX

Kanchan Joshi
2024-09-13T07:16:15Z

On 9/13/2024 2:06 AM, Bart Van Assche wrote:
> On 9/10/24 8:01 AM, Kanchan Joshi wrote:
>> diff --git a/include/linux/rw_hint.h b/include/linux/rw_hint.h
>> index b9942f5f13d3..ff708a75e2f6 100644
>> --- a/include/linux/rw_hint.h
>> +++ b/include/linux/rw_hint.h
>> @@ -21,4 +21,17 @@ enum rw_lifetime_hint {
>>   static_assert(sizeof(enum rw_lifetime_hint) == 1);
>>   #endif
>> +#define WRITE_HINT_TYPE_BIT    BIT(7)
>> +#define WRITE_HINT_VAL_MASK    (WRITE_HINT_TYPE_BIT - 1)
>> +#define WRITE_HINT_TYPE(h)    (((h) & WRITE_HINT_TYPE_BIT) ? \
>> +                TYPE_RW_PLACEMENT_HINT : TYPE_RW_LIFETIME_HINT)
>> +#define WRITE_HINT_VAL(h)    ((h) & WRITE_HINT_VAL_MASK)
>> +
>> +#define WRITE_PLACEMENT_HINT(h)    (((h) & WRITE_HINT_TYPE_BIT) ? \
>> +                 WRITE_HINT_VAL(h) : 0)
>> +#define WRITE_LIFETIME_HINT(h)    (((h) & WRITE_HINT_TYPE_BIT) ? \
>> +                 0 : WRITE_HINT_VAL(h))
>> +
>> +#define PLACEMENT_HINT_TYPE    WRITE_HINT_TYPE_BIT
>> +#define MAX_PLACEMENT_HINT_VAL    (WRITE_HINT_VAL_MASK - 1)
>>   #endif /* _LINUX_RW_HINT_H */
>
> The above macros implement a union of two 7-bit types in an 8-bit field.
> Wouldn't we be better of by using two separate 8-bit values such that we
> don't need the above macros?

I had considered that, but it requires two bytes of space. In inode,
bio, and request.
For example this change in inode:

@@ -674,7 +674,13 @@ struct inode {
spinlock_t i_lock; /* i_blocks, i_bytes, maybe
i_size */
unsigned short i_bytes;
u8 i_blkbits;
- u8 i_write_hint;
+ union {
+ struct {
+ enum rw_liftime_hint lifetime_hint;
+ u8 placement_hint;
+ };
+ u16 i_write_hint;
+ };

With this, generic propagation code will continue to use
inode->i_write_hint. And specific places (that care) can use either
lifetime_hint or placement_hint.

That kills the need of type-bit and above macros, but we don't have the
two bytes of space currently.