lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <0ef44e01-13a6-2519-bce2-075ca14a0cb9@huawei.com>
Date:   Thu, 31 Oct 2019 14:55:06 +0800
From:   Chao Yu <yuchao0@...wei.com>
To:     Jaegeuk Kim <jaegeuk@...nel.org>,
        "Theodore Y. Ts'o" <tytso@....edu>
CC:     Jonathan Corbet <corbet@....net>, <linux-doc@...r.kernel.org>,
        Gao Xiang <hsiangkao@....com>, <linux-kernel@...r.kernel.org>,
        <linux-f2fs-devel@...ts.sourceforge.net>
Subject: Re: [f2fs-dev] [PATCH] f2fs: bio_alloc should never fail

On 2019/10/31 0:33, Jaegeuk Kim wrote:
> On 10/30, Theodore Y. Ts'o wrote:
>> On Wed, Oct 30, 2019 at 11:50:37PM +0800, Gao Xiang wrote:
>>>
>>> So I'm curious about the original issue in commit 740432f83560
>>> ("f2fs: handle failed bio allocation"). Since f2fs manages multiple write
>>> bios with its internal fio but it seems the commit is not helpful to
>>> resolve potential mempool deadlock (I'm confused since no calltrace,
>>> maybe I'm wrong)...
>>
>> Two possibilities come to mind.  (a) It may be that on older kernels
>> (when f2fs is backported to older Board Support Package kernels from
>> the SOC vendors) didn't have the bio_alloc() guarantee, so it was
>> necessary on older kernels, but not on upstream, or (b) it wasn't
>> *actually* possible for bio_alloc() to fail and someone added the
>> error handling in 740432f83560 out of paranoia.
> 
> Yup, I was checking old device kernels but just stopped digging it out.
> Instead, I hesitate to apply this patch since I can't get why we need to
> get rid of this code for clean-up purpose. This may be able to bring
> some hassles when backporting to android/device kernels.

Jaegeuk,

IIUC, as getting hint from commit 740432f83560, are we trying to fix potential
deadlock like this?

In low memory scenario:

- f2fs_write_checkpoint()
 - block_operations()
  - f2fs_sync_node_pages()
   step 1) flush cold nodes, allocate new bio from mempool
   - bio_alloc()
    - mempool_alloc()
   step 2) flush hot nodes, allocate a bio from mempool
   - bio_alloc()
    - mempool_alloc()
   step 3) flush warm nodes, be stuck in below call path
   - bio_alloc()
    - mempool_alloc()
     - loop to wait mempool element release, as we only
       reserved memory for two bio allocation, however above
       allocated two bios never getting submitted.

#define BIO_POOL_SIZE 2

If so, we need avoid using bioset, or introducing private bioset, at least
enlarging mempool size to three (adapt to total log headers' number)...

Thanks,

> 
>>
>> (Hence my suggestion that in the ext4 version of the patch, we add a
>> code comment justifying why there was no error checking, to make it
>> clear that this was a deliberate choice.  :-)
>>
>> 						- Ted
> 
> 
> _______________________________________________
> Linux-f2fs-devel mailing list
> Linux-f2fs-devel@...ts.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
> .
> 

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ