Re: [PATCH v2] z3fold: fix the potential encode bug in encod_handle
On 2016/10/17 23:30, Dan Streetman wrote: > On Mon, Oct 17, 2016 at 8:48 AM, zhong jiangwrote: >> On 2016/10/17 20:03, Vitaly Wool wrote: >>> Hi Zhong Jiang, >>> >>> On Mon, Oct 17, 2016 at 3:58 AM, zhong jiang wrote: Hi, Vitaly About the following patch, is it right? Thanks zhongjiang On 2016/10/13 12:02, zhongjiang wrote: > From: zhong jiang > > At present, zhdr->first_num plus bud can exceed the BUDDY_MASK > in encode_handle, it will lead to the the caller handle_to_buddy > return the error value. > > The patch fix the issue by changing the BUDDY_MASK to PAGE_MASK, > it will be consistent with handle_to_z3fold_header. At the same time, > change the BUDDY_MASK to PAGE_MASK in handle_to_buddy is better. >>> are you seeing problems with the existing code? first_num should wrap around >>> BUDDY_MASK and this should be ok because it is way bigger than the number >>> of buddies. >>> >>> ~vitaly >>> >>> . >>> >> first_num plus buddies can exceed the BUDDY_MASK. is it right? > yes. > >> (first_num + buddies) & BUDDY_MASK may be a smaller value than first_num. > yes, but that doesn't matter; the value stored in the handle is never > accessed directly. > >> but (handle - zhdr->first_num) & BUDDY_MASK will return incorrect value >> in handle_to_buddy. > the subtraction and masking will result in the correct buddy number, > even if (handle & BUDDY_MASK) < zhdr->first_num. yes, I see. it is hard to read. > However, I agree it's nonobvious, and tying the first_num size to > NCHUNKS_ORDER is confusing - the number of chunks is completely > unrelated to the number of buddies. yes. indeed. > Possibly a better way to handle first_num is to limit it to the order > of enum buddy to the actual range of possible buddy indexes, which is > 0x3, i.e.: > > #define BUDDY_MASK (0x3) > > and > >unsigned short first_num:2; > > with that and a small bit of explanation in the encode_handle or > handle_to_buddy comments, it should be clear how the first_num and > buddy numbering work, including that overflow/underflow are ok (due to > the masking)... yes, It is better and clearer. Thanks for your relpy and advice. I will resend the patch. >> Thanks >> zhongjiang >> >> > . >
Re: [PATCH v2] z3fold: fix the potential encode bug in encod_handle
On 2016/10/17 23:30, Dan Streetman wrote: > On Mon, Oct 17, 2016 at 8:48 AM, zhong jiang wrote: >> On 2016/10/17 20:03, Vitaly Wool wrote: >>> Hi Zhong Jiang, >>> >>> On Mon, Oct 17, 2016 at 3:58 AM, zhong jiang wrote: Hi, Vitaly About the following patch, is it right? Thanks zhongjiang On 2016/10/13 12:02, zhongjiang wrote: > From: zhong jiang > > At present, zhdr->first_num plus bud can exceed the BUDDY_MASK > in encode_handle, it will lead to the the caller handle_to_buddy > return the error value. > > The patch fix the issue by changing the BUDDY_MASK to PAGE_MASK, > it will be consistent with handle_to_z3fold_header. At the same time, > change the BUDDY_MASK to PAGE_MASK in handle_to_buddy is better. >>> are you seeing problems with the existing code? first_num should wrap around >>> BUDDY_MASK and this should be ok because it is way bigger than the number >>> of buddies. >>> >>> ~vitaly >>> >>> . >>> >> first_num plus buddies can exceed the BUDDY_MASK. is it right? > yes. > >> (first_num + buddies) & BUDDY_MASK may be a smaller value than first_num. > yes, but that doesn't matter; the value stored in the handle is never > accessed directly. > >> but (handle - zhdr->first_num) & BUDDY_MASK will return incorrect value >> in handle_to_buddy. > the subtraction and masking will result in the correct buddy number, > even if (handle & BUDDY_MASK) < zhdr->first_num. yes, I see. it is hard to read. > However, I agree it's nonobvious, and tying the first_num size to > NCHUNKS_ORDER is confusing - the number of chunks is completely > unrelated to the number of buddies. yes. indeed. > Possibly a better way to handle first_num is to limit it to the order > of enum buddy to the actual range of possible buddy indexes, which is > 0x3, i.e.: > > #define BUDDY_MASK (0x3) > > and > >unsigned short first_num:2; > > with that and a small bit of explanation in the encode_handle or > handle_to_buddy comments, it should be clear how the first_num and > buddy numbering work, including that overflow/underflow are ok (due to > the masking)... yes, It is better and clearer. Thanks for your relpy and advice. I will resend the patch. >> Thanks >> zhongjiang >> >> > . >
Re: [PATCH v2] z3fold: fix the potential encode bug in encod_handle
On Mon, Oct 17, 2016 at 8:48 AM, zhong jiangwrote: > > On 2016/10/17 20:03, Vitaly Wool wrote: > > Hi Zhong Jiang, > > > > On Mon, Oct 17, 2016 at 3:58 AM, zhong jiang wrote: > >> Hi, Vitaly > >> > >> About the following patch, is it right? > >> > >> Thanks > >> zhongjiang > >> On 2016/10/13 12:02, zhongjiang wrote: > >>> From: zhong jiang > >>> > >>> At present, zhdr->first_num plus bud can exceed the BUDDY_MASK > >>> in encode_handle, it will lead to the the caller handle_to_buddy > >>> return the error value. > >>> > >>> The patch fix the issue by changing the BUDDY_MASK to PAGE_MASK, > >>> it will be consistent with handle_to_z3fold_header. At the same time, > >>> change the BUDDY_MASK to PAGE_MASK in handle_to_buddy is better. > > are you seeing problems with the existing code? first_num should wrap around > > BUDDY_MASK and this should be ok because it is way bigger than the number > > of buddies. > > > > ~vitaly > > > > . > > > first_num plus buddies can exceed the BUDDY_MASK. is it right? yes. > > (first_num + buddies) & BUDDY_MASK may be a smaller value than first_num. yes, but that doesn't matter; the value stored in the handle is never accessed directly. > > but (handle - zhdr->first_num) & BUDDY_MASK will return incorrect value > in handle_to_buddy. the subtraction and masking will result in the correct buddy number, even if (handle & BUDDY_MASK) < zhdr->first_num. However, I agree it's nonobvious, and tying the first_num size to NCHUNKS_ORDER is confusing - the number of chunks is completely unrelated to the number of buddies. Possibly a better way to handle first_num is to limit it to the order of enum buddy to the actual range of possible buddy indexes, which is 0x3, i.e.: #define BUDDY_MASK (0x3) and unsigned short first_num:2; with that and a small bit of explanation in the encode_handle or handle_to_buddy comments, it should be clear how the first_num and buddy numbering work, including that overflow/underflow are ok (due to the masking)... > > Thanks > zhongjiang > >
Re: [PATCH v2] z3fold: fix the potential encode bug in encod_handle
On Mon, Oct 17, 2016 at 8:48 AM, zhong jiang wrote: > > On 2016/10/17 20:03, Vitaly Wool wrote: > > Hi Zhong Jiang, > > > > On Mon, Oct 17, 2016 at 3:58 AM, zhong jiang wrote: > >> Hi, Vitaly > >> > >> About the following patch, is it right? > >> > >> Thanks > >> zhongjiang > >> On 2016/10/13 12:02, zhongjiang wrote: > >>> From: zhong jiang > >>> > >>> At present, zhdr->first_num plus bud can exceed the BUDDY_MASK > >>> in encode_handle, it will lead to the the caller handle_to_buddy > >>> return the error value. > >>> > >>> The patch fix the issue by changing the BUDDY_MASK to PAGE_MASK, > >>> it will be consistent with handle_to_z3fold_header. At the same time, > >>> change the BUDDY_MASK to PAGE_MASK in handle_to_buddy is better. > > are you seeing problems with the existing code? first_num should wrap around > > BUDDY_MASK and this should be ok because it is way bigger than the number > > of buddies. > > > > ~vitaly > > > > . > > > first_num plus buddies can exceed the BUDDY_MASK. is it right? yes. > > (first_num + buddies) & BUDDY_MASK may be a smaller value than first_num. yes, but that doesn't matter; the value stored in the handle is never accessed directly. > > but (handle - zhdr->first_num) & BUDDY_MASK will return incorrect value > in handle_to_buddy. the subtraction and masking will result in the correct buddy number, even if (handle & BUDDY_MASK) < zhdr->first_num. However, I agree it's nonobvious, and tying the first_num size to NCHUNKS_ORDER is confusing - the number of chunks is completely unrelated to the number of buddies. Possibly a better way to handle first_num is to limit it to the order of enum buddy to the actual range of possible buddy indexes, which is 0x3, i.e.: #define BUDDY_MASK (0x3) and unsigned short first_num:2; with that and a small bit of explanation in the encode_handle or handle_to_buddy comments, it should be clear how the first_num and buddy numbering work, including that overflow/underflow are ok (due to the masking)... > > Thanks > zhongjiang > >
Re: [PATCH v2] z3fold: fix the potential encode bug in encod_handle
On 2016/10/17 20:03, Vitaly Wool wrote: > Hi Zhong Jiang, > > On Mon, Oct 17, 2016 at 3:58 AM, zhong jiangwrote: >> Hi, Vitaly >> >> About the following patch, is it right? >> >> Thanks >> zhongjiang >> On 2016/10/13 12:02, zhongjiang wrote: >>> From: zhong jiang >>> >>> At present, zhdr->first_num plus bud can exceed the BUDDY_MASK >>> in encode_handle, it will lead to the the caller handle_to_buddy >>> return the error value. >>> >>> The patch fix the issue by changing the BUDDY_MASK to PAGE_MASK, >>> it will be consistent with handle_to_z3fold_header. At the same time, >>> change the BUDDY_MASK to PAGE_MASK in handle_to_buddy is better. > are you seeing problems with the existing code? first_num should wrap around > BUDDY_MASK and this should be ok because it is way bigger than the number > of buddies. > > ~vitaly > > . > I am curious about the z3fold implement, Thus, I am reviewing the code. Thanks zhongjiang
Re: [PATCH v2] z3fold: fix the potential encode bug in encod_handle
On 2016/10/17 20:03, Vitaly Wool wrote: > Hi Zhong Jiang, > > On Mon, Oct 17, 2016 at 3:58 AM, zhong jiang wrote: >> Hi, Vitaly >> >> About the following patch, is it right? >> >> Thanks >> zhongjiang >> On 2016/10/13 12:02, zhongjiang wrote: >>> From: zhong jiang >>> >>> At present, zhdr->first_num plus bud can exceed the BUDDY_MASK >>> in encode_handle, it will lead to the the caller handle_to_buddy >>> return the error value. >>> >>> The patch fix the issue by changing the BUDDY_MASK to PAGE_MASK, >>> it will be consistent with handle_to_z3fold_header. At the same time, >>> change the BUDDY_MASK to PAGE_MASK in handle_to_buddy is better. > are you seeing problems with the existing code? first_num should wrap around > BUDDY_MASK and this should be ok because it is way bigger than the number > of buddies. > > ~vitaly > > . > I am curious about the z3fold implement, Thus, I am reviewing the code. Thanks zhongjiang
Re: [PATCH v2] z3fold: fix the potential encode bug in encod_handle
On 2016/10/17 20:03, Vitaly Wool wrote: > Hi Zhong Jiang, > > On Mon, Oct 17, 2016 at 3:58 AM, zhong jiangwrote: >> Hi, Vitaly >> >> About the following patch, is it right? >> >> Thanks >> zhongjiang >> On 2016/10/13 12:02, zhongjiang wrote: >>> From: zhong jiang >>> >>> At present, zhdr->first_num plus bud can exceed the BUDDY_MASK >>> in encode_handle, it will lead to the the caller handle_to_buddy >>> return the error value. >>> >>> The patch fix the issue by changing the BUDDY_MASK to PAGE_MASK, >>> it will be consistent with handle_to_z3fold_header. At the same time, >>> change the BUDDY_MASK to PAGE_MASK in handle_to_buddy is better. > are you seeing problems with the existing code? first_num should wrap around > BUDDY_MASK and this should be ok because it is way bigger than the number > of buddies. > > ~vitaly > > . > first_num plus buddies can exceed the BUDDY_MASK. is it right? (first_num + buddies) & BUDDY_MASK may be a smaller value than first_num. but (handle - zhdr->first_num) & BUDDY_MASK will return incorrect value in handle_to_buddy. Thanks zhongjiang
Re: [PATCH v2] z3fold: fix the potential encode bug in encod_handle
On 2016/10/17 20:03, Vitaly Wool wrote: > Hi Zhong Jiang, > > On Mon, Oct 17, 2016 at 3:58 AM, zhong jiang wrote: >> Hi, Vitaly >> >> About the following patch, is it right? >> >> Thanks >> zhongjiang >> On 2016/10/13 12:02, zhongjiang wrote: >>> From: zhong jiang >>> >>> At present, zhdr->first_num plus bud can exceed the BUDDY_MASK >>> in encode_handle, it will lead to the the caller handle_to_buddy >>> return the error value. >>> >>> The patch fix the issue by changing the BUDDY_MASK to PAGE_MASK, >>> it will be consistent with handle_to_z3fold_header. At the same time, >>> change the BUDDY_MASK to PAGE_MASK in handle_to_buddy is better. > are you seeing problems with the existing code? first_num should wrap around > BUDDY_MASK and this should be ok because it is way bigger than the number > of buddies. > > ~vitaly > > . > first_num plus buddies can exceed the BUDDY_MASK. is it right? (first_num + buddies) & BUDDY_MASK may be a smaller value than first_num. but (handle - zhdr->first_num) & BUDDY_MASK will return incorrect value in handle_to_buddy. Thanks zhongjiang
Re: [PATCH v2] z3fold: fix the potential encode bug in encod_handle
Hi Zhong Jiang, On Mon, Oct 17, 2016 at 3:58 AM, zhong jiangwrote: > Hi, Vitaly > > About the following patch, is it right? > > Thanks > zhongjiang > On 2016/10/13 12:02, zhongjiang wrote: >> From: zhong jiang >> >> At present, zhdr->first_num plus bud can exceed the BUDDY_MASK >> in encode_handle, it will lead to the the caller handle_to_buddy >> return the error value. >> >> The patch fix the issue by changing the BUDDY_MASK to PAGE_MASK, >> it will be consistent with handle_to_z3fold_header. At the same time, >> change the BUDDY_MASK to PAGE_MASK in handle_to_buddy is better. are you seeing problems with the existing code? first_num should wrap around BUDDY_MASK and this should be ok because it is way bigger than the number of buddies. ~vitaly
Re: [PATCH v2] z3fold: fix the potential encode bug in encod_handle
Hi Zhong Jiang, On Mon, Oct 17, 2016 at 3:58 AM, zhong jiang wrote: > Hi, Vitaly > > About the following patch, is it right? > > Thanks > zhongjiang > On 2016/10/13 12:02, zhongjiang wrote: >> From: zhong jiang >> >> At present, zhdr->first_num plus bud can exceed the BUDDY_MASK >> in encode_handle, it will lead to the the caller handle_to_buddy >> return the error value. >> >> The patch fix the issue by changing the BUDDY_MASK to PAGE_MASK, >> it will be consistent with handle_to_z3fold_header. At the same time, >> change the BUDDY_MASK to PAGE_MASK in handle_to_buddy is better. are you seeing problems with the existing code? first_num should wrap around BUDDY_MASK and this should be ok because it is way bigger than the number of buddies. ~vitaly
Re: [PATCH v2] z3fold: fix the potential encode bug in encod_handle
Hi, Vitaly About the following patch, is it right? Thanks zhongjiang On 2016/10/13 12:02, zhongjiang wrote: > From: zhong jiang> > At present, zhdr->first_num plus bud can exceed the BUDDY_MASK > in encode_handle, it will lead to the the caller handle_to_buddy > return the error value. > > The patch fix the issue by changing the BUDDY_MASK to PAGE_MASK, > it will be consistent with handle_to_z3fold_header. At the same time, > change the BUDDY_MASK to PAGE_MASK in handle_to_buddy is better. > > Signed-off-by: zhong jiang > --- > mm/z3fold.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/mm/z3fold.c b/mm/z3fold.c > index 8f9e89c..e8fc216 100644 > --- a/mm/z3fold.c > +++ b/mm/z3fold.c > @@ -169,7 +169,7 @@ static unsigned long encode_handle(struct z3fold_header > *zhdr, enum buddy bud) > > handle = (unsigned long)zhdr; > if (bud != HEADLESS) > - handle += (bud + zhdr->first_num) & BUDDY_MASK; > + handle += (bud + zhdr->first_num) & PAGE_MASK; > return handle; > } > > @@ -183,7 +183,7 @@ static struct z3fold_header > *handle_to_z3fold_header(unsigned long handle) > static enum buddy handle_to_buddy(unsigned long handle) > { > struct z3fold_header *zhdr = handle_to_z3fold_header(handle); > - return (handle - zhdr->first_num) & BUDDY_MASK; > + return (handle - zhdr->first_num) & PAGE_MASK; > } > > /*
Re: [PATCH v2] z3fold: fix the potential encode bug in encod_handle
Hi, Vitaly About the following patch, is it right? Thanks zhongjiang On 2016/10/13 12:02, zhongjiang wrote: > From: zhong jiang > > At present, zhdr->first_num plus bud can exceed the BUDDY_MASK > in encode_handle, it will lead to the the caller handle_to_buddy > return the error value. > > The patch fix the issue by changing the BUDDY_MASK to PAGE_MASK, > it will be consistent with handle_to_z3fold_header. At the same time, > change the BUDDY_MASK to PAGE_MASK in handle_to_buddy is better. > > Signed-off-by: zhong jiang > --- > mm/z3fold.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/mm/z3fold.c b/mm/z3fold.c > index 8f9e89c..e8fc216 100644 > --- a/mm/z3fold.c > +++ b/mm/z3fold.c > @@ -169,7 +169,7 @@ static unsigned long encode_handle(struct z3fold_header > *zhdr, enum buddy bud) > > handle = (unsigned long)zhdr; > if (bud != HEADLESS) > - handle += (bud + zhdr->first_num) & BUDDY_MASK; > + handle += (bud + zhdr->first_num) & PAGE_MASK; > return handle; > } > > @@ -183,7 +183,7 @@ static struct z3fold_header > *handle_to_z3fold_header(unsigned long handle) > static enum buddy handle_to_buddy(unsigned long handle) > { > struct z3fold_header *zhdr = handle_to_z3fold_header(handle); > - return (handle - zhdr->first_num) & BUDDY_MASK; > + return (handle - zhdr->first_num) & PAGE_MASK; > } > > /*
[PATCH v2] z3fold: fix the potential encode bug in encod_handle
From: zhong jiangAt present, zhdr->first_num plus bud can exceed the BUDDY_MASK in encode_handle, it will lead to the the caller handle_to_buddy return the error value. The patch fix the issue by changing the BUDDY_MASK to PAGE_MASK, it will be consistent with handle_to_z3fold_header. At the same time, change the BUDDY_MASK to PAGE_MASK in handle_to_buddy is better. Signed-off-by: zhong jiang --- mm/z3fold.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/mm/z3fold.c b/mm/z3fold.c index 8f9e89c..e8fc216 100644 --- a/mm/z3fold.c +++ b/mm/z3fold.c @@ -169,7 +169,7 @@ static unsigned long encode_handle(struct z3fold_header *zhdr, enum buddy bud) handle = (unsigned long)zhdr; if (bud != HEADLESS) - handle += (bud + zhdr->first_num) & BUDDY_MASK; + handle += (bud + zhdr->first_num) & PAGE_MASK; return handle; } @@ -183,7 +183,7 @@ static struct z3fold_header *handle_to_z3fold_header(unsigned long handle) static enum buddy handle_to_buddy(unsigned long handle) { struct z3fold_header *zhdr = handle_to_z3fold_header(handle); - return (handle - zhdr->first_num) & BUDDY_MASK; + return (handle - zhdr->first_num) & PAGE_MASK; } /* -- 1.8.3.1
[PATCH v2] z3fold: fix the potential encode bug in encod_handle
From: zhong jiang At present, zhdr->first_num plus bud can exceed the BUDDY_MASK in encode_handle, it will lead to the the caller handle_to_buddy return the error value. The patch fix the issue by changing the BUDDY_MASK to PAGE_MASK, it will be consistent with handle_to_z3fold_header. At the same time, change the BUDDY_MASK to PAGE_MASK in handle_to_buddy is better. Signed-off-by: zhong jiang --- mm/z3fold.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/mm/z3fold.c b/mm/z3fold.c index 8f9e89c..e8fc216 100644 --- a/mm/z3fold.c +++ b/mm/z3fold.c @@ -169,7 +169,7 @@ static unsigned long encode_handle(struct z3fold_header *zhdr, enum buddy bud) handle = (unsigned long)zhdr; if (bud != HEADLESS) - handle += (bud + zhdr->first_num) & BUDDY_MASK; + handle += (bud + zhdr->first_num) & PAGE_MASK; return handle; } @@ -183,7 +183,7 @@ static struct z3fold_header *handle_to_z3fold_header(unsigned long handle) static enum buddy handle_to_buddy(unsigned long handle) { struct z3fold_header *zhdr = handle_to_z3fold_header(handle); - return (handle - zhdr->first_num) & BUDDY_MASK; + return (handle - zhdr->first_num) & PAGE_MASK; } /* -- 1.8.3.1