{"action":"created","issue":{"url":"https://api.github.com/repos/zfsonlinux/zfs/issues/3521","labels_url":"https://api.github.com/repos/zfsonlinux/zfs/issues/3521/labels{/name}","comments_url":"https://api.github.com/repos/zfsonlinux/zfs/issues/3521/comments","events_url":"https://api.github.com/repos/zfsonlinux/zfs/issues/3521/events","html_url":"https://github.com/zfsonlinux/zfs/pull/3521","id":90750293,"number":3521,"title":"Account for ashift when gathering buffers to be written to l2arc device","user":{"login":"behlendorf","id":148917,"avatar_url":"https://avatars.githubusercontent.com/u/148917?v=3","gravatar_id":"","url":"https://api.github.com/users/behlendorf","html_url":"https://github.com/behlendorf","followers_url":"https://api.github.com/users/behlendorf/followers","following_url":"https://api.github.com/users/behlendorf/following{/other_user}","gists_url":"https://api.github.com/users/behlendorf/gists{/gist_id}","starred_url":"https://api.github.com/users/behlendorf/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/behlendorf/subscriptions","organizations_url":"https://api.github.com/users/behlendorf/orgs","repos_url":"https://api.github.com/users/behlendorf/repos","events_url":"https://api.github.com/users/behlendorf/events{/privacy}","received_events_url":"https://api.github.com/users/behlendorf/received_events","type":"User","site_admin":false},"labels":[],"state":"closed","locked":false,"assignee":null,"milestone":null,"comments":3,"created_at":"2015-06-24T18:15:55Z","updated_at":"2015-06-26T08:25:51Z","closed_at":"2015-06-25T16:26:40Z","pull_request":{"url":"https://api.github.com/repos/zfsonlinux/zfs/pulls/3521","html_url":"https://github.com/zfsonlinux/zfs/pull/3521","diff_url":"https://github.com/zfsonlinux/zfs/pull/3521.diff","patch_url":"https://github.com/zfsonlinux/zfs/pull/3521.patch"},"body":"If we don't account for that, then we might end up overwriting disk\r\narea of buffers that have not been evicted yet, because l2arc_evict\r\noperates in terms of disk addresses.\r\n\r\nThe discrepancy between the write size calculation and the actual\r\nincrement to l2ad_hand was introduced in commit 3a17a7a9.\r\n\r\nThe change that introduced l2ad_hand alignment was almost correct\r\nas the write size was accumulated as a sum of rounded buffer sizes.\r\nSee commit illumos/illumos-gate@e14bb32.\r\n\r\nAlso, we now consistently use asize / a_sz for the allocated size and\r\npsize / p_sz for the physical size.  The latter accounts for a\r\npossible size reduction because of the compression, whereas the\r\nformer accounts for a possible subsequent size expansion because of\r\nthe alignment requirements.\r\n\r\nThe code still assumes that either underlying storage subsystems or\r\nhardware is able to do read-modify-write when an L2ARC buffer size is\r\nnot a multiple of a disk's block size.  This is true for 4KB sector disks\r\nthat provide 512B sector emulation, but may not be true in general.\r\nIn other words, we currently do not have any code to make sure that\r\nan L2ARC buffer, whether compressed or not, which is used for physical\r\nI/O has a suitable size.\r\n\r\nNote that currently the cache device utilization is calculated based\r\non the physical size, not the allocated size.  The same applies to\r\nl2_asize kstat. That is wrong, but this commit does not fix that.\r\nThe accounting problem was introduced partially in commit 3a17a7a9\r\nand partially in 3038a2b (accounting became consistent but in favour\r\nof the wrong size).\r\n\r\nPorting Notes:\r\n\r\nReworked to be C90 compatible and the 'write_psize' variable was\r\nremoved because it is now unused.\r\n\r\nReferences:\r\n  https://reviews.csiden.org/r/229/\r\n  https://reviews.freebsd.org/D2764\r\n\r\nPorted-by: kernelOfTruth <kerneloftruth@gmail.com>\r\nSigned-off-by: Brian Behlendorf <behlendorf1@llnl.gov>\r\nIssue #3400\r\nIssue #3433\r\nIssue #3451"},"comment":{"url":"https://api.github.com/repos/zfsonlinux/zfs/issues/comments/115573067","html_url":"https://github.com/zfsonlinux/zfs/pull/3521#issuecomment-115573067","issue_url":"https://api.github.com/repos/zfsonlinux/zfs/issues/3521","id":115573067,"user":{"login":"avg-I","id":1499700,"avatar_url":"https://avatars.githubusercontent.com/u/1499700?v=3","gravatar_id":"","url":"https://api.github.com/users/avg-I","html_url":"https://github.com/avg-I","followers_url":"https://api.github.com/users/avg-I/followers","following_url":"https://api.github.com/users/avg-I/following{/other_user}","gists_url":"https://api.github.com/users/avg-I/gists{/gist_id}","starred_url":"https://api.github.com/users/avg-I/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/avg-I/subscriptions","organizations_url":"https://api.github.com/users/avg-I/orgs","repos_url":"https://api.github.com/users/avg-I/repos","events_url":"https://api.github.com/users/avg-I/events{/privacy}","received_events_url":"https://api.github.com/users/avg-I/received_events","type":"User","site_admin":false},"created_at":"2015-06-26T08:25:51Z","updated_at":"2015-06-26T08:25:51Z","body":"@behlendorf @kernelOfTruth thank you and all who helped with testing, etc, very much!\r\nBTW, I have some follow-on work: https://reviews.freebsd.org/D2789\r\nBut it's not yet rebased on the latest ARC code."}}