Re: [PATCH 1/2] mm: move filemap_range_needs_writeback() into header

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 12/3/21 9:31 AM, Jens Axboe wrote:
> On 12/3/21 9:24 AM, Jens Axboe wrote:
>> On 12/3/21 9:16 AM, Matthew Wilcox wrote:
>>> On Fri, Dec 03, 2021 at 08:38:28AM -0700, Jens Axboe wrote:
>>>> +++ b/include/linux/fs.h
>>>
>>> fs.h is the wrong place for these functions; they're pagecache
>>> functionality, so they should be in pagemap.h.
>>>
>>>> +/* Returns true if writeback might be needed or already in progress. */
>>>> +static inline bool mapping_needs_writeback(struct address_space *mapping)
>>>> +{
>>>> +	return mapping->nrpages;
>>>> +}
>>>
>>> I don't like this function -- mapping_needs_writeback says to me that it
>>> tests a flag in mapping->flags.  Plus, it does exactly the same thing as
>>> !mapping_empty(), so perhaps ...
>>>
>>>> +static inline bool filemap_range_needs_writeback(struct address_space *mapping,
>>>> +						 loff_t start_byte,
>>>> +						 loff_t end_byte)
>>>> +{
>>>> +	if (!mapping_needs_writeback(mapping))
>>>> +		return false;
>>>
>>> just make this
>>> 	if (mapping_empty(mapping))
>>> 		return false;
>>>
>>> Other than that, no objections to making this static inline.
>>
>> Good idea, I'll make that change.
> 
> That does introduce a dependency from fs.h -> pagemap.h which isn't trivially
> resolvable...
> 
> What if we just rename the above funciton to mapping_has_pages() or something
> instead?

Or just drop the helper, to be honest. There are more tests for mapping->nrpages
right now than there are callers of this silly little helper.

-- 
Jens Axboe





[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Bugtraq]     [Linux OMAP]     [Linux MIPS]     [eCos]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux