On Thu, Feb 21, 2019 at 02:57:27PM +0000, Dan Robertson wrote: > > Thanks, it's been there since the first merge of scrub, but hard to hit. > > Yeah, very hard to hit. Debated submitting a patch, but the fix seemed > simple enough, so figured I'd send one in. > > Side note: I noticed most of the time ENOMEM is hit in fs/btrfs BUG or BUG_ON > is > called. Would it be better (more in line with the fs/btrfs style) to call BUG > or > BUG_ON e.g. BUG_ON(!sbio) instead of goto nomem at fs/btrfs/scrub.c:592?
No, we want to get rid of the BUG_ONs as a poor substitute for error handling. The ENOMEM case is usually a hard one to fix so you still can see a lot of them, but there are places where it's simple enough to rollback and return ENOMEM up to the original caller. Scrub in this case is restartable and if scrub_setup_ctx fails, it's at the time of start and the user is properly notified.