On Wed, Dec 22, 2010 at 03:41:13PM -0600, Alex Elder wrote: > > + error = xfs_alloc_read_agf(mp, NULL, agno, > > + XFS_ALLOC_FLAG_TRYLOCK, &agbp); > > + if (error || !agbp) { > > + if (error == EAGAIN) > > + error = 0; > > EAGAIN is ignored because it's an advisory interface, right? > How hard are we expected to try? What I really mean is, > is the benefit of FITRIM enough that we should try again > later when we can get a buffer or lock on it? That was the idea when I wrote this code. But back then we called it regularly from a kernel thread. For FITRIM it makes more sense to just remove the trylock. > I don't know where (or if) FITRIM is precisely documented. > But I question whether truncating down the start offset is > the correct thing to do. If the starting byte offset given > were not block-aligned, it seems like you should not assume > that the caller wanted the bytes below unmapped. (This is > a broader question, not related directly to your change.) > > Similarly, on the length it is probably best to truncate > it, because it avoids any bytes beyond the specified range > getting unmapped. (I.e., in my mind what you did is the > right way to do it.) But these interpretations are > dependent on the specific interpretation of FITRIM... Good question. Adding Lukas to the Cc. I tried to talk him into writing a manpage to document the interface better, but that's only been a few days before the holidays. This is something we should documented. I don't quite understand the need for the range interface anyway. > You don't update range anywhere, so the copyout below > is not really doing anything useful. However I think > it should stay, and the number of bytes actually > trimmed should be updated and returned to the user. > That seems to be what ext4 does (the only reference > I found at the moment for what FITRIM is supposed > to return). Yes, I guess I should update the range. _______________________________________________ xfs mailing list xfs@xxxxxxxxxxx http://oss.sgi.com/mailman/listinfo/xfs