On 2019/5/24 3:00, Andrew Morton wrote:
...
> On Thu, 23 May 2019 22:24:15 +0800 Aaron Lu <aaron.lu@xxxxxxxxxxxxxxxxx> wrote:
>> ...
>>
>> +static struct swap_extent *
>> +offset_to_swap_extent(struct swap_info_struct *sis, unsigned long offset)
>> +{
>> + struct swap_extent *se;
>> + struct rb_node *rb;
>> +
>> + rb = sis->swap_extent_root.rb_node;
>> + while (rb) {
>> + se = rb_entry(rb, struct swap_extent, rb_node);
>> + if (offset < se->start_page)
>> + rb = rb->rb_left;
>> + else if (offset >= se->start_page + se->nr_pages)
>> + rb = rb->rb_right;
>> + else
>> + return se;
>> + }
>> + /* It *must* be present */
>> + BUG_ON(1);
>
> I'm surprised this doesn't generate a warning about the function
Ah right, I'm also surprised after you mentioned.
This BUG_ON(1) here is meant to serve the same purpose as the
original code in map_swap_entry():
static sector_t map_swap_entry(swp_entry_t entry, struct block_device **bdev)
{
...
offset = swp_offset(entry);
start_se = sis->curr_swap_extent;
se = start_se;
for ( ; ; ) {
if (se->start_page <= offset &&
offset < (se->start_page + se->nr_pages)) {
return se->start_block + (offset - se->start_page);
}
se = list_next_entry(se, list);
sis->curr_swap_extent = se;
BUG_ON(se == start_se); /* It *must* be present */
}
}
I just copied the pattern and changed the condition to 1 without
much thought.
> failing to return a value. I guess the compiler figured out that
> BUG_ON(non-zero-constant) is equivalent to BUG(), which is noreturn.
>
> Let's do this?
Yes, it doesn't make much sense to use BUG_ON when the condition
is 1...Thanks for the cleanup.
>
> --- a/mm/swapfile.c~mm-swap-use-rbtree-for-swap_extent-fix
> +++ a/mm/swapfile.c
> @@ -218,7 +218,7 @@ offset_to_swap_extent(struct swap_info_s
> return se;
> }
> /* It *must* be present */
> - BUG_ON(1);
> + BUG();
> }
>
