Re: [GIT PULL] Writeback fix

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

 



On 12/10/22 10:44?AM, Linus Torvalds wrote:
> On Sat, Dec 10, 2022 at 7:36 AM Jens Axboe <axboe@xxxxxxxxx> wrote:
>>
>> Just a single writeback fix from Jan, for sanity checking adding freed
>> inodes to lists.
> 
> That's what the commit message says too, but that's not what the patch
> actually *does*.
> 
> It also does that unexplained
> 
> +       if (inode->i_state & I_FREEING) {
> +               list_del_init(&inode->i_io_list);
> +               wb_io_lists_depopulated(wb);
> +               return;
> +       }
> 
> that is new.
> 
> And yes, it has a link: in the commit message. And yes, I followed the
> link in case it had some background.
> 
> And dammit, it's ANOTHER of those stupid pointless and worthless links
> that just links to the patch submission, and has NO ADDITIONAL
> INFORMATION.

I agree that sometimes they are useless, but sometimes there's
discussion on the patch as well in that link. And ideally the patch
itself, when sent to the list, should include the link to the report, if
any. Then you'd get both.

> Those links are actively detrimental. Stop it. I just wasted time
> hoping that there would be some information about why the patch was
> sent to me this late in the game. Instead, I just wated time on it.
> 
> I pulled this and then unpulled it. I'm very very annoyed. This patch
> has an actively misleading commit message, has no explanation for why
> it's so critical that it needs to be sent, and has a useless link to
> garbage.

Just to be clear, this was deliberately held for the 6.2 merge window,
but I can also see that I completely missed that in the pull request.
Sorry about that, that should've been clear.

> Fix the damn explanation to actually match the change. Fix the damn
> link to point to something *useful* like the error report or
> something.

I'll let Jan resubmit this one, just disregard this pull request and
we'll send a new one during the merge window.

> And STOP WASTING EVERYBODY'S TIME with these annoying links that I
> keep hoping would explain something and give useful background to the
> change and instead just are a source of constant disappointment.

For me, applying patches is done by using a script, which is why all
patches get the link. I do think it's worth having the link, because
some of them will indeed have useful discussion. Is it worth it to
manually have to deal with that, in case there's nothing there?

-- 
Jens Axboe




[Index of Archives]     [Linux Ext4 Filesystem]     [Union Filesystem]     [Filesystem Testing]     [Ceph Users]     [Ecryptfs]     [NTFS 3]     [AutoFS]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux Cachefs]     [Reiser Filesystem]     [Linux RAID]     [NTFS 3]     [Samba]     [Device Mapper]     [CEPH Development]

  Powered by Linux