linux-kernel - Re: [PATCH v14 022/138] mm/filemap: Add __folio_lock_or

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 PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 10 Aug 2021 18:08:03 +0200
From:   Vlastimil Babka <vbabka@...e.cz>
To:     "Matthew Wilcox (Oracle)" <willy@...radead.org>,
        linux-kernel@...r.kernel.org
Cc:     linux-mm@...ck.org, linux-fsdevel@...r.kernel.org,
        Christoph Hellwig <hch@....de>,
        Jeff Layton <jlayton@...nel.org>,
        "Kirill A . Shutemov" <kirill.shutemov@...ux.intel.com>,
        William Kucharski <william.kucharski@...cle.com>
Subject: Re: [PATCH v14 022/138] mm/filemap: Add __folio_lock_or_retry()

On 7/15/21 5:35 AM, Matthew Wilcox (Oracle) wrote:
> Convert __lock_page_or_retry() to __folio_lock_or_retry().  This actually
> saves 4 bytes in the only caller of lock_page_or_retry() (due to better
> register allocation) and saves the 14 byte cost of calling page_folio()
> in __folio_lock_or_retry() for a total saving of 18 bytes.  Also use
> a bool for the return type.
> 
> Signed-off-by: Matthew Wilcox (Oracle) <willy@...radead.org>
> Reviewed-by: Christoph Hellwig <hch@....de>
> Acked-by: Jeff Layton <jlayton@...nel.org>
> Acked-by: Kirill A. Shutemov <kirill.shutemov@...ux.intel.com>
> Reviewed-by: William Kucharski <william.kucharski@...cle.com>

Acked-by: Vlastimil Babka <vbabka@...e.cz>

Nit:

> --- a/mm/filemap.c
> +++ b/mm/filemap.c
> @@ -1625,48 +1625,46 @@ static int __folio_lock_async(struct folio *folio, struct wait_page_queue *wait)
>  
>  /*
>   * Return values:
> - * 1 - page is locked; mmap_lock is still held.
> - * 0 - page is not locked.
> + * true - folio is locked; mmap_lock is still held.
> + * false - folio is not locked.
>   *     mmap_lock has been released (mmap_read_unlock(), unless flags had both
>   *     FAULT_FLAG_ALLOW_RETRY and FAULT_FLAG_RETRY_NOWAIT set, in
>   *     which case mmap_lock is still held.
>   *
>   * If neither ALLOW_RETRY nor KILLABLE are set, will always return 1

s/1/true/ ? :)

> - * with the page locked and the mmap_lock unperturbed.
> + * with the folio locked and the mmap_lock unperturbed.
>   */
> -int __lock_page_or_retry(struct page *page, struct mm_struct *mm,
> +bool __folio_lock_or_retry(struct folio *folio, struct mm_struct *mm,
>  			 unsigned int flags)
>  {
> -	struct folio *folio = page_folio(page);
> -
>  	if (fault_flag_allow_retry_first(flags)) {
>  		/*
>  		 * CAUTION! In this case, mmap_lock is not released
>  		 * even though return 0.
>  		 */
>  		if (flags & FAULT_FLAG_RETRY_NOWAIT)
> -			return 0;
> +			return false;
>  
>  		mmap_read_unlock(mm);
>  		if (flags & FAULT_FLAG_KILLABLE)
>  			folio_wait_locked_killable(folio);
>  		else
>  			folio_wait_locked(folio);
> -		return 0;
> +		return false;
>  	}
>  	if (flags & FAULT_FLAG_KILLABLE) {
> -		int ret;
> +		bool ret;
>  
>  		ret = __folio_lock_killable(folio);
>  		if (ret) {
>  			mmap_read_unlock(mm);
> -			return 0;
> +			return false;
>  		}
>  	} else {
>  		__folio_lock(folio);
>  	}
>  
> -	return 1;
> +	return true;
>  }
>  
>  /**