On Wed, Feb 10, 2021 at 06:57:30AM +0000, Chaitanya Kulkarni wrote:
> On 2/9/21 22:25, ira.weiny@xxxxxxxxx wrote:
> > From: Ira Weiny <ira.weiny@xxxxxxxxx>
> >
> > Add VM_BUG_ON bounds checks to ensure the newly lifted and created page
> > memory operations do not result in corrupted data in neighbor pages and
> > to make them consistent with zero_user().[1][2]
> >
> I did not understand this, in my tree :-
>
> zero_user()
> -> zero_user_segments()
>
> which uses BUG_ON(), the commit log says add VM_BUG_ON(), isn't that
> inconsistent withwhat is there in zero_user_segments() which uses BUG_ON() ?
>
> Also, this patch uses BUG_ON() which doesn't match the commit log that says
> ADD VM_BUG_ON(),
Oops, yea that 'consistent with zero_user()' was carried over from the BUG_ON
commit comment in the original patch...
The comment should be deleted.
But I'm going to wait because Christoph prefers BUG_ON...
Ira
>
> Did I interpret the commit log wrong ?
>
> [1]
> void zero_user_segments(struct page *page, unsigned start1, unsigned end1,
> 365 unsigned start2, unsigned end2)
> 366 {
> 367 unsigned int
> i;
>
> 368
> 369 BUG_ON(end1 > page_size(page) || end2 > page_size(page));
> 370
> 371 for (i = 0; i < compound_nr(page); i++) {
> 372 void *kaddr = NULL;
> 373
> 374 if (start1 < PAGE_SIZE || start2 < PAGE_SIZE)
> 375 kaddr = kmap_atomic(page + i);
> 376
> 377 if (start1 >= PAGE_SIZE) {
> 378 start1 -= PAGE_SIZE;
> 379 end1 -= PAGE_SIZE;
> 380 } else {
> 381 unsigned this_end = min_t(unsigned, end1,
> PAGE_SIZE);
> 382
> 383 if (end1 > start1)
> 384 memset(kaddr + start1, 0, this_end -
> start1);
> 385 end1 -= this_end;
> 386 start1 = 0;
> 387 }
> 388
> 389 if (start2 >= PAGE_SIZE) {
> 390 start2 -= PAGE_SIZE;
> 391 end2 -= PAGE_SIZE;
> 392 } else {
> 393 unsigned this_end = min_t(unsigned, end2,
> PAGE_SIZE);
> 394
> 395 if (end2 > start2)
> 396 memset(kaddr + start2, 0, this_end -
> start2);
> 397 end2 -= this_end;
> 398 start2 = 0;
> 399 }
> 400
> 401 if (kaddr) {
> 402 kunmap_atomic(kaddr);
> 403 flush_dcache_page(page + i);
> 404 }
> 405
> 406 if (!end1 && !end2)
> 407 break;
> 408 }
> 409
> 410 BUG_ON((start1 | start2 | end1 | end2) != 0);
> 411 }
> 412 EXPORT_SYMBOL(zero_user_segments);
>
>
>
