From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751913AbdF3AnX (ORCPT ); Thu, 29 Jun 2017 20:43:23 -0400 Received: from mail-qt0-f195.google.com ([209.85.216.195]:34027 "EHLO mail-qt0-f195.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751625AbdF3AnW (ORCPT ); Thu, 29 Jun 2017 20:43:22 -0400 Subject: Re: [PATCH] cma: fix calculation of aligned offset To: Andrew Morton Cc: Gregory Fong , Angus Clark , Laura Abbott , Vlastimil Babka , Greg Kroah-Hartman , Lucas Stach , Catalin Marinas , Shiraz Hashim , Jaewon Kim , "open list:MEMORY MANAGEMENT" , open list References: <20170628170742.2895-1-opendmb@gmail.com> <20170629134810.3a5b09dbdea001cca72080ce@linux-foundation.org> From: Doug Berger Message-ID: Date: Thu, 29 Jun 2017 17:43:18 -0700 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.1.1 MIME-Version: 1.0 In-Reply-To: <20170629134810.3a5b09dbdea001cca72080ce@linux-foundation.org> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 06/29/2017 01:48 PM, Andrew Morton wrote: > On Wed, 28 Jun 2017 10:07:41 -0700 Doug Berger wrote: > >> The align_offset parameter is used by bitmap_find_next_zero_area_off() >> to represent the offset of map's base from the previous alignment >> boundary; the function ensures that the returned index, plus the >> align_offset, honors the specified align_mask. >> >> The logic introduced by commit b5be83e308f7 ("mm: cma: align to >> physical address, not CMA region position") has the cma driver >> calculate the offset to the *next* alignment boundary. In most cases, >> the base alignment is greater than that specified when making >> allocations, resulting in a zero offset whether we align up or down. >> In the example given with the commit, the base alignment (8MB) was >> half the requested alignment (16MB) so the math also happened to work >> since the offset is 8MB in both directions. However, when requesting >> allocations with an alignment greater than twice that of the base, >> the returned index would not be correctly aligned. >> >> Also, the align_order arguments of cma_bitmap_aligned_mask() and >> cma_bitmap_aligned_offset() should not be negative so the argument >> type was made unsigned. > > The changelog doesn't describe the user-visible effects of the bug. It > should do so please, so that others can decide which kernel(s) need the fix. > > Since the bug has been there for three years, I'll assume that -stable > backporting is not needed. > I'm afraid I'm confused by what you are asking me to do since it appears that you have already signed-off on this patch. The direct user-visible effect of the bug is that if the user requests a CMA allocation that is aligned with a granule that is more than twice the base alignment of the CMA region she will receive an allocation that does not have that alignment. As I indicated to Gregory, the follow-on consequences of the address not satisfying the required alignment depend on why the alignment was requested. In our case it was a system crash, but it could also manifest as data corruption on a network interface for example. In general I would expect it to be unusual for anyone to request an allocation alignment that is larger than the CMA base alignment which is probably why the bug has been hiding for three years. Thanks for your support with this and let me know what more you would like from me. -Doug