[developer] Re: [openzfs/openzfs] 7531 Assign correct flags to prefetch buffers (#222)
Closed #222 via 468008cba949f0fe6a2c79e7510a9cd230bd0e1d. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/openzfs/openzfs/pull/222#event-1334183945 -- openzfs-developer Archives: https://openzfs.topicbox.com/groups/developer/discussions/Tc1b045964aee4a5f-Mfd985c4f6cbb603696d5c09e Powered by Topicbox: https://topicbox.com
[developer] Re: [openzfs/openzfs] 7531 Assign correct flags to prefetch buffers (#222)
prakashsurya approved this pull request. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/openzfs/openzfs/pull/222#pullrequestreview-71054599 -- openzfs-developer Archives: https://openzfs.topicbox.com/groups/developer/discussions/Tc1b045964aee4a5f-Mb8f43a4b69201d29598be44b Powered by Topicbox: https://topicbox.com
[developer] Re: [openzfs/openzfs] 7531 Assign correct flags to prefetch buffers (#222)
Any idea when this tiny patch will be merged ? Thank you very much -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/openzfs/openzfs/pull/222#issuecomment-338464850 -- openzfs-developer Archives: https://openzfs.topicbox.com/groups/developer/discussions/Tc1b045964aee4a5f-M50439c774187414dbb7d4de0 Powered by Topicbox: https://topicbox.com
[developer] Re: [openzfs/openzfs] 7531 Assign correct flags to prefetch buffers (#222)
Thank you Matt I'm really glad :) -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/openzfs/openzfs/pull/222#issuecomment-324652140 -- openzfs-developer Archives: https://openzfs.topicbox.com/groups/developer/discussions/T45c103905470ca47-Mdc11db540a6c7fafdf2792c9 Powered by Topicbox: https://topicbox.com
[developer] Re: [openzfs/openzfs] 7531 Assign correct flags to prefetch buffers (#222)
benrubson commented on this pull request. > @@ -347,6 +347,12 @@ boolean_t dbuf_is_metadata(dmu_buf_impl_t *db); (dbuf_is_metadata(_db) && \ ((_db)->db_objset->os_secondary_cache == ZFS_CACHE_METADATA))) Not sure then I really see how to construct such a function without breaking `DBUF_IS_L2CACHEABLE*` working :| -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/openzfs/openzfs/pull/222#discussion_r135030283 -- openzfs-developer Archives: https://openzfs.topicbox.com/groups/developer/discussions/T45c103905470ca47-Mc193e6da2fe16a4dd332c33b Powered by Topicbox: https://topicbox.com
[developer] Re: [openzfs/openzfs] 7531 Assign correct flags to prefetch buffers (#222)
ahrens approved this pull request. It would be nice to clean up DBUF_IS_L2CACHEABLE so that the logic isn't duplicated with DBUF_IS_L2CACHEABLE_IMPL, but I won't make that a requirement. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/openzfs/openzfs/pull/222#pullrequestreview-58233975 -- openzfs-developer Archives: https://openzfs.topicbox.com/groups/developer/discussions/T45c103905470ca47-M6b97be160509309204e066a5 Powered by Topicbox: https://topicbox.com
[developer] Re: [openzfs/openzfs] 7531 Assign correct flags to prefetch buffers (#222)
> l2arc_noprefetch=0 is useless because prefetched buffers are not > ARC_FLAG_L2CACHE flagged If a buffer is prefetched but not hit, it will not be L2ARC eligible, because it will not be flagged accordingly, so l2arc_noprefetch=0 has no effect on this buffer here I agree; I'll accept this PR based on that. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/openzfs/openzfs/pull/222#issuecomment-324480521 -- openzfs-developer Archives: https://openzfs.topicbox.com/groups/developer/discussions/T45c103905470ca47-M2364c22e53b4ee52d3a5ba5e Powered by Topicbox: https://topicbox.com
[developer] Re: [openzfs/openzfs] 7531 Assign correct flags to prefetch buffers (#222)
ahrens commented on this pull request. > @@ -347,6 +347,12 @@ boolean_t dbuf_is_metadata(dmu_buf_impl_t *db); (dbuf_is_metadata(_db) && \ ((_db)->db_objset->os_secondary_cache == ZFS_CACHE_METADATA))) I guess that doesn't really work (it doesn't evaluate to `result`). You'd have to convert this to an inline function. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/openzfs/openzfs/pull/222#discussion_r134884758 -- openzfs-developer Archives: https://openzfs.topicbox.com/groups/developer/discussions/T45c103905470ca47-M549846f56994ea1a83f11614 Powered by Topicbox: https://topicbox.com
[developer] Re: [openzfs/openzfs] 7531 Assign correct flags to prefetch buffers (#222)
Test passed, thank you for having launched it @prakashsurya I'm pretty convinced prefetched buffers should be `ARC_FLAG_L2CACHE` flagged (what this PR currently does), as `l2arc_noprefetch` user tunable will then control whether or not these buffers can be L2 backed. This gives a nice performance improvement, as shown in the [bug report](https://www.illumos.org/issues/7531). FreeBSD has _dtrace_, but I never used it, so I've no idea how to easily track the path of these buffers which, according to you @ahrens, should have been flagged by `arc_read`. And what about prefetched buffers which are evicted before being read ? Thank you very much for your help -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/openzfs/openzfs/pull/222#issuecomment-324242895 -- openzfs-developer Archives: https://openzfs.topicbox.com/groups/developer/discussions/T45c103905470ca47-M76ad212c02718b172bd31c68 Powered by Topicbox: https://topicbox.com
[developer] Re: [openzfs/openzfs] 7531 Assign correct flags to prefetch buffers (#222)
@prakashsurya could you restart the test please ? Many thx -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/openzfs/openzfs/pull/222#issuecomment-322043862 -- openzfs-developer Archives: https://openzfs.topicbox.com/groups/developer/discussions/T45c103905470ca47-Ma0b88bfa6c95ec72d06b444c Powered by Topicbox: https://topicbox.com
