On 11/7/23 5:02 PM, Mina Almasry wrote: > On Mon, Nov 6, 2023 at 1:02 PM Stanislav Fomichev <sdf@xxxxxxxxxx> wrote: >> >> On 11/05, Mina Almasry wrote: >>> +static inline bool page_is_page_pool_iov(const struct page *page) >>> +{ >>> + return (unsigned long)page & PP_DEVMEM; >>> +} >> >> Speaking of bpf: one thing that might be problematic with this PP_DEVMEM >> bit is that it will make debugging with bpftrace a bit (more) >> complicated. If somebody were trying to get to that page_pool_iov from >> the frags, they will have to do the equivalent of page_is_page_pool_iov, >> but probably not a big deal? (thinking out loud) > > Good point, but that doesn't only apply to bpf I think. I'm guessing > even debugger drgn access to the bv_page in the frag will have trouble > if it's actually accessing an iov with LSB set. > > But this is not specific to this use for LSB pointer trick. I think > all code that currently uses LSB pointer trick will have similar > troubles. In this context my humble vote is that we get such big > upside from reducing code churn that it's reasonable to tolerate such > side effects. +1 > > I could alleviate some of the issues by teaching drgn to do the right > thing for devmem/iovs... time permitting. > Tools like drgn and crash have to know when the LSB trick is used - e.g., dst_entry - and handle it when dereferencing pointers.